Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: make eps parameter const & functions constexpr in RectUtil #2195

Merged
merged 1 commit into from
Jun 19, 2024

Conversation

kkafar
Copy link
Member

@kkafar kkafar commented Jun 18, 2024

Description

The eps parameter in compareFrameSizes should be const.

Both functions can be constexpr so I've marked them as such.

Note

When doing a bit of research for this PR I've learned that constexpr implies inline linkage (so also bending the ODR rule) for functions (and it seems that since C++17 also for namespace scoped variables) and must admit I was not aware of this before, thus I've come to conclusion that it will be ok to leave inline keyword there, so that the intent of inline linkage is clear.

Test code and steps to reproduce

Build Android / iOS

Checklist

  • Ensured that CI passes

@kkafar kkafar marked this pull request as draft June 18, 2024 22:31
@kkafar kkafar marked this pull request as ready for review June 19, 2024 08:43
@kkafar kkafar merged commit cb20152 into main Jun 19, 2024
6 of 7 checks passed
@kkafar kkafar deleted the @kkafar/missing-const-specifier branch June 19, 2024 10:31
alduzy pushed a commit that referenced this pull request Jun 28, 2024
)

## Description

The `eps` parameter in `compareFrameSizes` should be `const`. 

Both functions can be `constexpr` so I've marked them as such.

> [!note]
When doing a bit of research for this PR I've learned that [`constexpr`
implies `inline` linkage (so also bending the ODR
rule)](https://stackoverflow.com/a/14391320/8635522) for functions (and
it seems that since C++17 also for namespace scoped variables) and must
admit I was not aware of this before, thus I've come to conclusion that
it will be ok to leave `inline` keyword there, so that the intent of
`inline` linkage is clear.


## Test code and steps to reproduce

Build Android / iOS

## Checklist

- [ ] Ensured that CI passes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants