`-werror=true` doesn't get forwarded to the `*_link_args`

[1ace]

Describe the bug meson setup -werror=true doesn't enable *_link_args=-Werror, which is obviously not the expected behaviour :)

To Reproduce Build Mesa as of 4f50eba5a5719d76fa41; it contains linker warnings that had been missed until now because of this bug, so you can see that meson doesn't pass -werror through.

Expected behavior :stuck_out_tongue_winking_eye:

system parameters

• what meson --version 1.0.0

[eli-schwartz]

I'm not sure how this is supposed to be a bug when it was never intended to do anything to link args in the first place.

Moreover, the ld program doesn't accept a -Werror option, and for that matter it doesn't have an --error option either. If you try to pass one as gcc -Werror foo.o -o foo it will simply ignore the argument entirely, it's not passed to the forked linker... because -W options are exclusive to the compile stage. If you try to pass it via -Wl,-Werror, the linker errors out during argument parsing and objects that no such option exists, "please read the --help text".

To turn this around, how about we instead ask the question: what linker options would you like to be passed, such that:

• -Wl,{option} is actually allowed by the linker
• your goal to discover link-time issues which you haven't specified, and haven't linked to, are discovered

To Reproduce Build Mesa as of 4f50eba5a5719d76fa41; it contains linker warnings that had been missed until now because of this bug

After manually tracking down this commit, it turns out that this only modifies .cpp files, and neither the attached issue nor the MR it's part of have any discussion whatsoever, let alone discussion about linkers?

[eli-schwartz]

Expected behavior :stuck_out_tongue_winking_eye:

If the documentation wording is bothersome, documentation patches accepted.

[okias]

we need the -Wl,--fatal-warnings for the linker, I guess?

[1ace]

Moreover, the ld program doesn't accept a -Werror option, and for that matter it doesn't have an --error option either.

Ah, I tried c_linker_args=-Werror on my machine without thinking and it did what I expected, and I didn't check the docs before posting this issue. @okias is right: the linker flag is called --fatal-warnings.

After manually tracking down this commit, it turns out that this only modifies .cpp files, and neither the attached issue nor the MR it's part of have any discussion whatsoever, let alone discussion about linkers?

I was just pointing to some commit that predates any fix that we'll figure out, so that someone looking at this issue after we've fixed these warnings will still be able to reproduce the problem; there's nothing special about this commit other than that.

If the documentation wording is bothersome, documentation patches accepted.

Sure, I could send a patch to change "Treat warnings as errors" to "Treat compiler warnings (but not linker warnings) as errors", but do all C compilers split these two, or do some compiler treat linker warnings as errors already? What about other languages that meson support?
I think making it work is a better solution than coming up with a list of exceptions and documenting these, unless of course there's a reason why we can't do it :slightly_smiling_face:

[1ace]

Just as an update, I was actually right the first time, both exist, but -Werror is what -werror=true should enable.

https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/20375#note_1745417

[okias]

@eli-schwartz annual one-year ping, do you have any input on this (documentation :disappointed: ) issue?

[eli-schwartz]

I actually flip-flopped on this and think we should pass -Wl,--fatal-warnings.

The initial ask about passing -Werror to the linker is confusing and the underlying issue was actually that in LTO compiles, you are officially supposed to pass "all compiler flags to the linker as well", so not so much -Werror as "all c_args, and all buildtype args, and all warning_level args, etc." This is because LTO actually uses lto-wrapper to compile all files after producing intermediate bytecode by the compiler, and doesn't really document what options affect the bytecode generation vs the lto-wrapper stage.

[thesamesam]

For completeness: we handled the LTO vs -Werror part with https://github.com/mesonbuild/meson/pull/12750. Not sure if we want/need another bug for generally passing all compiler flags down to link-stage as well, which the GCC docs at least recommend...

[1ace]

Makes sense for LTO needing all the compiler args, but in the non-LTO case do you still think meson setup -werror=true should not treat linker warnings as error? I don't understand why the exception to an otherwise coherent logic 🤷

[eli-schwartz]

I'm not sure what you mean, my most recent comment I thought made it clear that I no longer think anything of the sort?

[1ace]

I'm sorry, I guess I misread your comment, I thought what you were saying was only for the LTO case. I'm glad I was wrong 😊

[eli-schwartz]

-Werror is not a linker flag at all. It provides meaningful value when passed to the compiler in "linker driver mode", but only in the LTO case

--fatal-warnings is a linker flag, passed via -Wl,--fatal-warnings, and provides meaningful value any time the linker sees it.

As for what's changed -- I now believe I made a mistake in closing this issue for two reasons:

• since the discussion originally talked about -Werror specifically but didn't mention LTO at all, I mistakenly thought that there was no use case for passing -Werror to the linker, and didn't think about LTO (an oversight since I myself use -Werror=* passed via $LDFLAGS). It's simply a mistake to not to pass that through for LTO, and it was also previously reported years ago but with specific mention of LTO, and we fixed that a while ago as noted above by Sam.
• I actually forgot that --fatal-warnings existed and didn't notice the comment mentioning it either. And I've come around on the idea that meson -D werror=true doesn't say it's specific to compilers, and that it's reasonable to use it for anything (so, linkers too).