mesonbuild / meson

The Meson Build System
http://mesonbuild.com
Apache License 2.0
5.53k stars 1.61k forks source link

Cannot disable warnings in subproject -- warning_level should offer option to pass `-w` to GCC and Clang #8636

Open Tachi107 opened 3 years ago

Tachi107 commented 3 years ago

In the release notes for Meson 0.50.0 is stated that a warning level of 0 does not enable any static analysis checks. GCC and Clang have a flag that does just that, -w, but it seems that Meson doesn't pass it to the compiler.

eli-schwartz commented 3 years ago

I don't see how this is a bug. warning_level=0 suppresses meson's built-in option to add warnings to the build, but if you've manually passed your own in add_project_arguments() or via CFLAGS then those should still be used rather than being actively purged from the build definition.

i.e. there are three states of warning: "do nothing (0), add warnings (>0), remove externally managed warnings (n/a)" and you're recommending that meson should make only the latter 2 options be possible.

Given that -w cannot even be overridden by adding additional warnings later on, that's a pretty weighty and opinionated thing to do, and it's completely inappropriate as an answer to someone merely trying to (for whatever reason) avoid using -Wall -Winvalid-pch in particular.

Tachi107 commented 3 years ago

Ok, so "0" means "use the default warning level of whatever compiler you are using", and not "none", even though the docs explicitly say that warning_level=0 means "none". (Set the warning level. From 0 = none to 3 = highest).

You're right in saying that users need this "don't do anything" option, but it is also true that the docs are a bit misleading and at the moment users don't have the option to disable warnings (a thing that would be particularly useful for subprojects).

Maybe add a warning_level=-1? Doesn't look that great, but I don't see alternatives :/ GCC/Clang have -w, MSVC has /w.

Also @eli-schwartz, could you please avoid closing issues without even waiting for a response? That's not really nice...

eli-schwartz commented 3 years ago

Users do have the option to tell the compiler to disable all past and future warnings. e.g. use meson setup -Dc_args='-w' or add_project_arguments('-w')

It seems pretty obvious to me that "don't enable any warnings" means the null set passed to the compiler, not "scan for manually enabled external warnings and disable them too". -w is not the null set, it is something rather than nothing.

In no world should anyone have expected -Dwarning_level=0 to have higher priority to disable generalized levels of warnings than -Dc_args=... or add_project_arguments() has to enable a finely crafted set of handpicked specialized ones.

could you please avoid closing issues without even waiting for a response? That's not really nice...

The ticket isn't locked for commenting? Closing a ticket indicates the person closing it believes that future responses won't change anything and the ticket should leave the pending queue of things to do... tickets can always be reopened if that changes.

Tachi107 commented 3 years ago

Ok, I agree. But there's still the issue of disabling warnings per subproject. Let's say that I'm using a subproject that manually enables some warnings for GCC-like compilers (-Wconversion) and I want to disable them; the only way to do that is passing -w to the subproject, but the only relevant option that I can pass to subprojects is warning_level. Something like warning_level=-1 would be really useful.

Do I have to open another issue?

eli-schwartz commented 3 years ago

Disabling warnings in a subproject is an interesting angle that unfortunately wasn't mentioned in the OP.

And apparently, it turns out you cannot use -Dsubproject:c_args=-w, since c_args is not per subproject. So that shoots down my initial thought on how to do this. Which means adding a warning_level that does it, would be an understandable desire.

eli-schwartz commented 3 years ago

It might be easier to add warning_level=-1, but then again, it might be prettier to add c_args per subproject. idk

Tachi107 commented 3 years ago

Looking at the issue tracker it seems that specifying *_args per subproject is something that has been needed/wanted for quite some time.

Even if implementing this using warning_level could be easier, I think what maybe using c_args would be better; passing warning-related options using c_args is a sort of hack, but a negative warning_level would also look like a hack, so if I had to choose I would use an already present hack rather than introducing a new one.

But maybe I'm wrong, who knows

tristan957 commented 1 year ago

@eli-schwartz is this closeable?

punytroll commented 1 year ago

I don't think it should be! I have need to disable a specific warning in a subproject, that is not disabled when I'm just specifying 'warning_level=0'. Currently I need to set 'werror=false' in order to make the build continue ... but then I get a warning displayed while compiling and that is not acceptable. I need to set '-Wno-stringop-overflow' and it won't get set, if I modify cpp_args.

tristan957 commented 1 year ago

Real solution I guess is to actually make all options that make sense available to subprojects. Baggage...

xclaesse commented 1 year ago

<lang>_args and <lang>_link_args definitely should be per subproject. They currently aren't purely because it's tricky to do from an implementation POV. Patches welcome of course.

punytroll commented 1 year ago

Is there a precedent for subproject-specific options? Maybe I can cobble together something ...

eli-schwartz commented 1 year ago

Yes, there's a bunch of them and making this work like that is a known pending issue.

tristan957 commented 1 year ago

@punytroll look at werror, warning_level, or default_library

sammyj85 commented 9 months ago

I too was surprised to find warning_level=0 did not add -w to give no warnings.

The "you can just do add_project_arguments('-w') yourself" argument is similar to saying "you don't need warning_level=1, because you can just add -Wall yourself". I thought the whole point of Meson's options were so you didn't need to add your own compiler specific flags.

I really like the warning_level=-1 to fix this problem in a compiler-independent manner.

At the very least, the documentation should be fixed so that warning_level=0 means "default" instead of "none".

<lang>_args can be per subproject as an independent issue.

dcbaker commented 9 months ago

I would be happy to review patches to address either or both of those issues

johnnovak commented 5 months ago

At the very least, the documentation should be fixed so that warning_level=0 means "default" instead of "none".

+1, it's super misleading. Things like this should be clearly documented, not just hoping or assuming the user will figure it out. It's reasonable to think setting this to zero means "turn off all warnings".

I thought the whole point of Meson's options were so you didn't need to add your own compiler specific flags.

Exactly my idea too.

tristan957 commented 5 months ago

We recently added a warning_level=everything. We should probably just add a warning_level=none.

eli-schwartz commented 5 months ago

But none is the null set, and that's already covered by 0 as I mentioned above.

It seems pretty obvious to me that "don't enable any warnings" means the null set passed to the compiler, not "scan for manually enabled external warnings and disable them too". -w is not the null set, it is something rather than nothing.

If we were going to add a dedicated word value it should be warning_level=suppress instead.

tristan957 commented 5 months ago

That is a better word, indeed

sammyj85 commented 5 months ago

But none is the null set, and that's already covered by 0 as I mentioned above.

Again, "none" means different to "default". The level of warnings I want is 'none' - this means the same as "the amount of warnings I want is none", which means something different to "the level of warnings I want is 'default'".

image

https://mesonbuild.com/Builtin-options.html#core-options

Suppress is a decent term.

eli-schwartz commented 5 months ago

The default meson value is 1, not 0, which means "default" isn't what you get when no flags are passed (i.e. the compiler default).

Hence why I dislike both "none" and "default" and opted for "suppress". :D

In terms of the documentation, it was suggested to be fixed in December and we've said we are happy to accept patches that clarify the documentation to better express intent, independent of any code changes made to meson's implementation of warning_level. Although it seems no one has had a chance to reword the docs yet.

sammyj85 commented 4 months ago

Yes, 0 = "compiler default" instead of "none".

lf- commented 1 month ago

semi related (primarily in case search engines are being bad): the fixed bug that it used to be impossible to set warning level at all: https://github.com/mesonbuild/meson/issues/5466