Hi!
On Mon, Jun 02, 2025 at 08:37:19PM +0200, Martin Uecker wrote:
> Am Montag, dem 02.06.2025 um 13:19 -0500 schrieb Segher Boessenkool:
> > On Mon, Jun 02, 2025 at 05:50:08PM +0200, Martin Uecker wrote:
> > > According to the discussion in the bugzilla there seems to be
> > > some consensus to activate the warning for -Wextra (I am also
> > > looking into implementing the suggested improvements that may
> > > make it suitable fo r-Wall). When making this change, I also
> > > noticed that it was not working for -Wc++-compat due to a typo.
> >
> > Do you have any data showing that -Wextra is good for this warning
> > (and that -Wall is not)? -Wall should have essentially no false
> > positives, -Wextra is allowed to be a bit more annoying, but the
> > requirements for the two are essentially the same, just the limit
> > "this is *too* annoying" shifted a bit.
>
> It would cause false positives, e.g. occasionally for the "goto fail"
> idiom in C, so I think it is not acceptable for -Wall. I based
> this also on the comments in the bug report.
Yeah, likely. That information should be in the commit message, and
the proposed commit message should be part of this patch submission :-)
> > Why do this in -Wc++-compat at all? You don't say. Well, you say that
> > is a bugfix, so it should be a separate (and trivial) patch.
>
> It is a hard error in C++. It is also already documented as being
> activated by -Wc++-compat.
Ah, the C++ standard requires this? That again then is information that
belongs in the commit messsage :-)
> I guess the trivial bug fix ("LangEnabledby -> LangEnabledBy")
Ah, now I finally see the typo :-)
> could be a separate "obviously correct" patch if there is no consensus
> to add the warning to -Wextra.
It is so much easier to do this as separate patches, even if they are
tiny patches.
> Finally, I missed a tiny fix to "libbacktrace/elf.c" where this warning
> would trigger with -Wextra.
Better fix that as well then :-)
Thank you!
Segher
