[cfe-dev] Warning when calling virtual functions from constructor/desctructor?

[Aaron Ballman via cfe-dev]

On Mon, Jan 14, 2019 at 1:59 PM John McCall via cfe-dev
<cfe-dev at lists.llvm.org> wrote:
>
> On 11 Jan 2019, at 11:23, Arnaud Bienner wrote:
> > > There's a fairly substantial difference between warnings that target
> > patterns
> >> that are *inarguably* bad, like the std::move problem (which in some
> > sense is
> >> a language defect that people just need to be taught how to work
> >> around),
> > and
> >> warnings that target code patterns that might be confusing or which
> >> have a
> >> higher-than-normal chance of being a typo. -Wparentheses is the
> >> classic
> >> example here
> >
> > Yes, and this warning is definitely like -Wparentheses: something that
> > could be wrong, but is not necessarily.
> > Still, I think it will catch tricky bugs.
>
> I agree.
>
> > What should we do about this warning/patch?
> > Deactivate it by default, with its own flag? without putting it in
> > Wall/extra?
> > Or wait for something like your proposal to be implemented?
> > Or just forget about it for now and simply wait for more feedback from
> > the
> > mailing before deciding?
>
> I think it should go in, default-off, and we should implement my
> proposal

AIUI, experience has shown that off-by-default warning flags almost
never get enabled by users. I don't think we should have a new,
default off warnings unless there is a strong use case suggesting
someone will actually enable it.

~Aaron

> as a way of eventually making it default-on.  Unfortunately, I don't
> really
> have time to implement my proposal right now; I'm way backed up as it
> is.
>
> John.
>
>
> >
> > Arnaud
> >
> >
> >
> >
> >
> > Le mer. 9 janv. 2019 à 01:14, John McCall <jmccall at apple.com> a
> > écrit :
> >
> >> On 8 Jan 2019, at 15:03, Arthur O'Dwyer wrote:
> >>
> >> I was thinking of `-Wreturn-std-move`, which is -Wmove/-Wmost/-Wall
> >> but not
> >> always-on.
> >>
> >> I honestly don't know why this isn't default-on.
> >>
> >> Grepping the code for DefaultIgnore, I see that -Wfor-loop-analysis
> >> is
> >> another example (but 4 years old).
> >>
> >> This is a harder call. At any rate, I think my point stands that this
> >> is
> >> not
> >> "frequent".
> >>
> >> There's a fairly substantial difference between warnings that target
> >> patterns
> >> that are *inarguably* bad, like the std::move problem (which in some
> >> sense is
> >> a language defect that people just need to be taught how to work
> >> around),
> >> and
> >> warnings that target code patterns that might be confusing or which
> >> have a
> >> higher-than-normal chance of being a typo. -Wparentheses is the
> >> classic
> >> example here: it unquestionably catches a common mistake that C
> >> unfortunately
> >> otherwise masks, but it's still perennially controversial because the
> >> assign-and-test idiom is so common in C programming, and there are a
> >> lot of
> >> people who still swear by reversing the equality test (0 == foo)
> >> instead
> >> of
> >> relying on the warning.
> >>
> >> John.
> >>
> >> On Tue, Jan 8, 2019 at 2:37 PM John McCall <jmccall at apple.com> wrote:
> >>
> >> On 8 Jan 2019, at 13:45, Arthur O'Dwyer wrote:
> >>
> >> On Tue, Jan 8, 2019 at 1:05 PM John McCall via cfe-dev <
> >> cfe-dev at lists.llvm.org> wrote:
> >>
> >> On 8 Jan 2019, at 8:37, Arnaud Bienner wrote:
> >>
> >> I realized I didn't put "DefaultIgnore" on this warning.
> >> I'm not experienced enough with clang to know what's the best way to
> >> deal
> >> with new warnings, but my feeling is that it would be sensible to
> >> have
> >>
> >> this
> >>
> >> new warning DefaultIgnore for now, in -Wall, and make it default
> >> once we
> >> have some feedback from the community: while not all C++ projects
> >> use
> >>
> >> -Wall
> >>
> >> (or -Wextra) I believe enough do to give us a chance to get some
> >>
> >> feedback.
> >>
> >> What do you think?
> >>
> >> We generally don't add things to -Wall. That's why I went into my
> >> whole
> >> spiel
> >> about versioning: I think it's a conversation we need to have before
> >> we're
> >> ready to accept this as a warning that's anything but hidden
> >> permanently
> >> behind its own opt-in flag.
> >>
> >> John: Wha? Clang *frequently* adds things to -Wall! -Wall includes
> >> -Wmost
> >> which includes a bunch of other categories, so while we don't often
> >> put new
> >> diagnostics *directly* under -Wall, pretty much every "reasonable"
> >> diagnostic eventually winds up in there somehow — which is the
> >> intent.
> >>
> >> I don't think we "frequently" add things to -Wall or -Wmost. We do
> >> somewhat
> >> frequently add warnings that are unconditionally default-on, but the
> >> groups
> >> have a conventional meaning that we don't generally touch. What
> >> recently-added
> >> warnings are you thinking of that are not default-on but which are
> >> included
> >> in a group like -Wall or -Wmost?
> >>
> >> John.
> >>
> >>
>
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev