Gcc compiler warning flags setting #22
Conversation
README.md
Outdated
| minimal_warnings = ["-Wall", "-Wno-error=deprecated-declarations"], | ||
| strict_warnings = ["-Wextra", "-Wpedantic"], | ||
| minimal_warnings = ["-Wno-error=deprecated-declarations"], | ||
| strict_warnings = ["-Wno-bool-compare"], |
There was a problem hiding this comment.
this does sound very complex, as users would need to figure out in which list which warning is contained.
There was a problem hiding this comment.
Discussion: Disallow disabling minimal_warnings, or any warning from minimal_warnings
There was a problem hiding this comment.
I added a new configuration file called gcc_defs.bzl in the extensions folder. This file contains settings used by the GCC toolchain, including lists of compiler warning flags that has to be used for all SCORE modules.
FYI: This is only a first proposal and has to be aligned/discussed.
Currently, we define three default warning feature sets:
- minimal_warnings: Users cannot disable any of these warnings.
- strict_warnings: Users can disable this feature if needed.
- treat_warnings_as_errors: Converts all warnings to errors using -Werror except -Wno-error=deprecated-declarations.
This feature is enabled by default and cannot be disabled.
To make additional configuration easier to the user, we provide a single interface for adding or removing compiler warning flags: the additional_warnings feature.
Users can use this feature to supply extra flags or disable non-protected warnings using -Wno-.
README.md
Outdated
| gcc = use_extension("@score_toolchains_gcc//extentions:gcc.bzl", "gcc") | ||
| gcc.extra_features( | ||
| features = [ | ||
| "minimal_warnings", |
There was a problem hiding this comment.
I'm not sure, but is it like this?
| "minimal_warnings", | |
| # minimal_warnings are enabled by default | |
| # strict_warnings are enabled by default | |
| # but for some reason we want to not treat them as errors: |
There was a problem hiding this comment.
I agree . The user does not need to add the features as they are already enabled by default. Nevertheless, he can disable them using the -<feature-name> (e.g. -minimal_warnings)
extentions/gcc.bzl
Outdated
| "-Wreorder", | ||
| "-Wnarrowing" | ||
| ], | ||
| "treat_warnings_as_errors": [] |
There was a problem hiding this comment.
Does the empty list here mean that -Werror is set? I am assuming we want errors as warnings on, and then I guess -Werror should be in this empty list?
There was a problem hiding this comment.
-Werror is not set by default.
treat_warnings_as_errors is empty so that warnings are not treated as errors by default. I did not want that during integration ( merge with main) that consumers are getting build errors for every warning they have.
It is also possible to enable -Werror by default and consumers disable the features using -treat_warnings_as_errors.
what do you think?
There was a problem hiding this comment.
At least "-Wno-error=deprecated-declarations" should be set here because the whole point of deprecations is to enable a smooth transition and not to break anyones build.
There was a problem hiding this comment.
I have set by default treat_warnings_as_errors to -Werror and -Wno-error=deprecated-declarations so that warnings will be converted to errors except deprecated-declarations .
|
Nice, did you already try them out with lola or baselibs? |
@FScholPer : yes, I tried it with baselibs repo for the bazel targets //score/datetime_converter and //score/filesystem and I can see some warnings. |
NEOatNHNG
left a comment
NEOatNHNG
left a comment
There was a problem hiding this comment.
-Wno-error=deprecated-declarations needs more consideration IMHO. Normally it should be set whenever you set -Werror
extentions/gcc.bzl
Outdated
| "-Wreorder", | ||
| "-Wnarrowing" | ||
| ], | ||
| "treat_warnings_as_errors": [] |
There was a problem hiding this comment.
At least "-Wno-error=deprecated-declarations" should be set here because the whole point of deprecations is to enable a smooth transition and not to break anyones build.
|
@AyJebri how is the state about the discussion and the pr? |
AyJebri
force-pushed
the
gcc_compiler_flags_setting
branch
from
f3c0fd0 to
0e2494e
Compare
@FScholPer: Sorry for the delayed reply, I pushed my changes based on the review remarks and I added my comments. |
AlexanderLanin
left a comment
AlexanderLanin
left a comment
There was a problem hiding this comment.
LGTM; Worth a try. Looking forward for some modules trying to use this.
github-project-automation
bot
moved this from Draft
to Done
in INF - Infrastructure Community
6 participants
Related issue: Define compiler flags for gcc compiler
This PR centralizes GCC warning flags configuration for improved consistency, maintainability, and reuse across the SCORE Project modules.
By default, the toolchain activates the following features using predefined compiler flags:
minimal_warnings(e.g., -Wall, -Wextra)strict_warnings(e.g., -Wpedantic)treat_warnings_as_errors(e.g., -Werror)Consumers can disable features via
-<feature_name>and customize behavior with additional flags or-Wno-options.