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

float_common.h: fix possible misuse of comma operator #214

Merged
merged 2 commits into from
Jul 10, 2023

Conversation

Coeur
Copy link
Contributor

@Coeur Coeur commented Jun 12, 2023

Xcode 15 is showing warnings when including fast_float. See screenshot.
Capture d’écran 2023-06-12 à 21 03 12

That warning says that the comma is meant to return a value, but that value is unused, so that code style is discouraged. I've reverted the code style to the previous one (partial revert of #180), and replaced the remaining commas with semicolons.

@lemire
Copy link
Member

lemire commented Jun 12, 2023

@leni536 Would you review? It is a reversion of a change you made, I think.

@@ -220,15 +220,13 @@ struct value128 {
/* Helper C++11 constexpr generic implementation of leading_zeroes */
fastfloat_really_inline constexpr
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

constexpr needs to be changed to FASTFLOAT_CONSTEXPR14 here.

The comma abuse was to make this function constexpr from C++11 (it requires constexpr function bodies to only consist of a single return statement). I think it's OK to revert this, I don't have high ambitions to port constexpr functionality all the way back to C++11.

Copy link
Contributor

@leni536 leni536 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change constexpr to FASTFLOAT_CONSTEXPR14 on the affected function.

@lemire
Copy link
Member

lemire commented Jun 12, 2023

@leni536 Thanks.

@Coeur Can you make the proposed fix?

@Coeur
Copy link
Contributor Author

Coeur commented Jun 12, 2023

@lemire done :)

@Coeur Coeur requested a review from leni536 June 12, 2023 20:31
@lemire lemire merged commit 33a2f8f into fastfloat:main Jul 10, 2023
@Coeur Coeur deleted the coeur/leading_zeroes_generic branch July 20, 2023 20:56
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.

3 participants