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

Document compiler warning_level project option #3275

Closed
ptomato opened this issue Mar 21, 2018 · 15 comments
Closed

Document compiler warning_level project option #3275

ptomato opened this issue Mar 21, 2018 · 15 comments

Comments

@ptomato
Copy link
Contributor

ptomato commented Mar 21, 2018

Searching for it in the documentation turns up nothing. It cost me quite a lot of source-diving to find out whether 1 or 3 was the most strict warning level.

@nirbheek nirbheek changed the title Document warning_level Document compiler warning_level project option Mar 21, 2018
@mwleeds
Copy link

mwleeds commented Sep 3, 2019

I think this can be closed? The documentation is here: https://mesonbuild.com/Builtin-options.html#

@ptomato
Copy link
Contributor Author

ptomato commented Sep 3, 2019

Excellent, thanks!

@ptomato ptomato closed this as completed Sep 3, 2019
@le-jzr
Copy link

le-jzr commented Jun 5, 2020

So, what do those levels actually mean? 3 being "the most strict" doesn't really say much, when I don't know which warnings are enabled. That would have been fine in the past, but with current version, meson actually spews warnings about "please use this vague option instead of manually using -Wall -Wextra", and I find that risky when it's not even documented what it's meant to mean exactly.

@philippludwig
Copy link

I agree about that. I have read the GCC manpage and have a pretty exact idea which warnings I would like to enable.

Furthermore, there should be an easy copy&paste snippet on the documentation page. If I write warning_level(3), I get:

meson.build:2:0: ERROR: Unknown function "warning_level".

And I don’t find any indication on how this option is being used correctly.

Therefore, I would like to suggest that the warning WARNING: Consider using the builtin warning_level option instead of adding warning flags by hand. is removed.

I assume that this feature is intended as some compiler-agnostic warning setting, but when I don’t care about other compilers, just let me set my flags as I would like without printing something like this.

@eli-schwartz
Copy link
Member

Worse is the fact that:

  • warning_level 1 adds -Wall,
    -warning_level 2 adds warning_level 1 + -Wextra,
  • warning_level 3 adds warning_level 2 + -Wpedantic

But some projects want -Wall and -Wpedantic, but refuse to add -Wextra because it's not a stable definition. It's therefore impossible to use the meson option. So the warning is useless, as it will warn you about things you cannot change.

@smac89
Copy link

smac89 commented Oct 31, 2020

Worse is the fact that:

* warning_level 1 adds `-Wall`,
  -warning_level 2 adds warning_level 1 + `-Wextra`,

* warning_level 3 adds warning_level 2 + `-Wpedantic`

But some projects want -Wall and -Wpedantic, but refuse to add -Wextra because it's not a stable definition. It's therefore impossible to use the meson option. So the warning is useless, as it will warn you about things you cannot change.

It would be nice if the above was documented to include the corresponding warning flags for different languages

@eli-schwartz
Copy link
Member

I submitted #7973 to stop meson from reprimanding you if you manually add -Wpedantic, did not (currently) try to document the flags each level add though.

@shemminger
Copy link

What about -Werror?

@eli-schwartz
Copy link
Member

meson.build:11: WARNING: Consider using the built-in werror option instead of using "-Werror".

Given there is a dedicated option just for Werror, I think it's perfectly reasonable that people should use that built-in option. There is no case where using it would also have the side effect of adding things you don't want to add, like there was with -Wpedantic.

coreboot-org-bot pushed a commit to flashrom/flashrom that referenced this issue Apr 29, 2022
Makefile options were more restrictive and produced more
warnings. This patch adds missing warning options into
meson build.

Makefile also has -Wall and -Wextra specified explicitly,
however this is covered by warning_level=2 which is already
set in meson.build. warning_level info:
mesonbuild/meson#3275

There are few warning options that are present in meson,
but not in Makefile. These are left as is.

TEST=ninja test shows no warnings and tests pass

Change-Id: Id401bfd642dc3c13d85bd9a2dba56ada38714c25
Signed-off-by: Anastasia Klimchuk <aklm@chromium.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/58561
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Thomas Heijligen <src@posteo.de>
@zippy2
Copy link

zippy2 commented Dec 12, 2022

Given there is a dedicated option just for Werror, I think it's perfectly reasonable that people should use that built-in option.

I beg to differ. There are projects which want to turn on -Werror when building from a git repo (i.e. when .git dir exists) but not when building from a release tarball as the latter maybe run on plethora of systems unavailable to developers and thus may fail build needlessly.

If we want projects to use builtin we need to give them an opportunity to set it outside of project().

@eli-schwartz
Copy link
Member

That will fail the build needlessly when users build from a git checkout.

@zippy2
Copy link

zippy2 commented Dec 12, 2022

Which is acceptable, because that's usually the case for advanced users building their own package (and they probably know ho to report this back to the project in question). Majority of users use distro shipped binary packages -> no problem.

@eli-schwartz
Copy link
Member

  • It's nowhere near true to say that only advanced users build from git checkouts.
  • Even advanced users cannot use their advanced knowledge to report the bug and then, having done their "duty", build the software anyway and ignore the warning. There's no option to disable -Werror hardcoded in the build files.
  • Claiming the majority of users use distro shipped binary packages implies that distros actually ship all software as binary packages, but they don't. They ship lots of popular software, and some unpopular software, but there is always stuff that users need to build themselves in order to use it. This ties back into point 1.

@Consolatis
Copy link

Consolatis commented Jan 31, 2023

  • warning_level 1 adds -Wall,
  • warning_level 2 adds warning_level 1 + -Wextra,
  • warning_level 3 adds warning_level 2 + -Wpedantic

Is this documented somewhere?
https://mesonbuild.com/Builtin-options.html only states From 0 = none to everything = highest.

On a side note: It seems neither none nor everything are the correct definitions of what that setting actually does.

@xclaesse
Copy link
Member

Maybe we should deprecate numerical values, and have more proper names. Like -Dwarning_level=all,pedantic. Probably would require some approximative mapping for compilers other than gcc/clang.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests