[PATCH] D69145: Give readability-redundant-member-init an option IgnoreBaseInCopyConstructors to avoid breaking code with gcc -Werror=extra

Roman Lebedev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Nov 18 14:01:55 PST 2019


lebedev.ri added a comment.

In D69145#1750529 <https://reviews.llvm.org/D69145#1750529>, @aaron.ballman wrote:

> In D69145#1748733 <https://reviews.llvm.org/D69145#1748733>, @lebedev.ri wrote:
>
> > In D69145#1748716 <https://reviews.llvm.org/D69145#1748716>, @poelmanc wrote:
> >
> > > In D69145#1715611 <https://reviews.llvm.org/D69145#1715611>, @mgehre wrote:
> > >
> > > > Exactly due to the issue you are fixing here, we ended up disabling the complete check because we didn't want to live with the warnings it produced on -Wextra.
> > > >  Therefore, I'm actually strongly in favor to enable the option by default.
> > > >
> > > > When others see that clang-tidy fixits introduce warnings (with -Wextra) or even break their build (with -Werror), they might not look into check options, but just disable the check directly.
> > >
> > >
> > > Just pinging to see if anyone has any thoughts on moving forward with this. Thanks in advance for any feedback!
> >
> >
> > If this would default to off i'd signoff.
>
>
> It sounds like you and @mgehre would like a different default for the new option?


I suppose. It's not unlike "clang -Wall is not supposed to match gcc's -Wall" issue.
Everyone may expect different behavior. Defaulting to on (don't warn) isn't misguided,
there's merit in not changing the code if it would break some other compiler the code supports.

But the other way around is also true, what if that gcc's diagnostic is not enabled,
or gcc is just not used? One then would need to actually know that there is an option
that should be flipped. **Here** i would prefer to have to disable something
if it's unwanted rather than somehow finding out that something can be enabled..

As a more meaningful data point, a similar relaxation to another check was added in D70165 <https://reviews.llvm.org/D70165>,
and there the default is to still warn - so not following the precedent seems inconsistent to me.

> Personally, I don't have a strong opinion on the default, but



> I do think we should have the option so that users can control the behavior so it doesn't conflict between clang-tidy and gcc.

Yes, i don't disagree that having *an ability* to control this is good.


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69145/new/

https://reviews.llvm.org/D69145





More information about the cfe-commits mailing list