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

More compiler warnings, and sanitizers, in CMake builds #239

Merged
merged 9 commits into from
Sep 20, 2024
Merged

Conversation

snej
Copy link
Contributor

@snej snej commented Sep 13, 2024

Added new CMake options:

  • FLEECE_WARNINGS_HARDCORE enables all compiler warnings except for a few dozen that are too onerous, plus -Werror.
  • FLEECE_SANITIZE enables the Clang Address and UB sanitizers.

Currently these are only supported in Clang builds, and ignored with other compilers.

Also made source changes to fix some of the newly-enabled warnings.

An inline function (not method!) in a header file shouldn't be
declared `static` or it'll be copied into every compilation unit
that uses it.
- Removed `using namespace` at global scope in headers (it pollutes
  the global namespace for anyone including those headers),
  then added it in the .cc files that include those headers.
- Some 3rd party headers trigger warnings, so wrapped those in pragmas
  to suppress them.
- Marked functions that always throw as `[[noreturn]]`.
- Definitions of `operator""` should have a space after the quotes.
- Some template instantiations needed to be declared explicitly in
  headers.
- Don't use 0 as a synonym of nullptr.
@borrrden
Copy link
Member

This is probably going to cause more warnings on non clang platforms due to unknown pragmas (clang diagnostic) but it's really just a drop in the bucket when it comes time to actually tackle that and we may well switch to clang everywhere first.

@snej snej merged commit 6aa9014 into master Sep 20, 2024
3 checks passed
@snej snej deleted the warnings branch September 20, 2024 22:40
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