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

[lib] Make lib compatible with -Wfall-through excepting legacy #2796

Merged
merged 1 commit into from
Sep 23, 2021

Conversation

terrelln
Copy link
Contributor

Switch to a macro ZSTD_FALLTHROUGH; instead of a comment. On supported
compilers this uses an attribute, otherwise it becomes a comment.

This is necessary to be compatible with clang's -Wfall-through, and
gcc's -Wfall-through=2 which don't support comments. Without this the
linux build emits a bunch of warnings.

Also add a test to CI to ensure that we don't regress.

@Cyan4973
Copy link
Contributor

This feature was also recently added to xxhash,
and I remember it wasn't completely trivial to ensure it would work correctly with a broad range of compilers.
The macro initialization ended up being a bit complex :
https://github.com/Cyan4973/xxHash/blob/dev/xxhash.h#L598
That being said, it works now. Might be useful for inspiration.

@terrelln
Copy link
Contributor Author

It looks like this section can be simplified with __has_attribute(__fallthrough__) as I'm doing in zstd. Clang always supported it, and gcc started supporting __has_attribute() before it supported __fallthrough__.

We can definitely add on the section that uses the C/C++ language attribute [[fallthrough]] though.

@Cyan4973
Copy link
Contributor

Cyan4973 commented Sep 23, 2021

Apparently, the macro determination code in xxhash uses __has_c_attribute(), instead of __has_attribute().
Maybe because it's looking for fallthrough, instead of __fallthrough__.

@terrelln
Copy link
Contributor Author

Apparently, the macro determination code in xxhash uses __has_c_attribute(), instead of __has_attribute().

__has_c_attribute() is to detect support for [[fallthrough]], where __has_attribute() detects support for __attribute__((__fallthrough__)).

# if __has_attribute(__fallthrough__)
# define ZSTD_FALLTHROUGH __attribute__((__fallthrough__))
# else
# define ZSTD_FALLTHROUGH /* fall-through */
Copy link
Contributor

Choose a reason for hiding this comment

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

I presume this will not add a comment where this macro is invoked ?

@terrelln terrelln force-pushed the linux-fixes branch 3 times, most recently from fce08a7 to 640a81c Compare September 23, 2021 17:48
Switch to a macro `ZSTD_FALLTHROUGH;` instead of a comment. On supported
compilers this uses an attribute, otherwise it becomes a comment.

This is necessary to be compatible with clang's `-Wfall-through`, and
gcc's `-Wfall-through=2` which don't support comments. Without this the
linux build emits a bunch of warnings.

Also add a test to CI to ensure that we don't regress.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants