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

[John McCall via cfe-dev]

On 8 Jan 2019, at 8:37, Arnaud Bienner wrote:
> Thanks for the detailed answer.
>
>> The code pattern is definitely not always harmful. Most importantly, the
>> behavior is only confusing if the method is actually overridden in a
> subclass.
>
> In this regard, note that my patch won't warn if the class is final.

Sure, that's a very reasonable improvement.

> Otherwise, indeed it might work as expected *now* but won't once
> reimplemented in a subclass. Qualifying the call make things clearer.

I don't disagree, but this doesn't diminish the fact that the behavior is
actually innocuous today (and it will probably be re-compiled / re-checked
after such a subclass is introduced).

>> The best workaround is to name the current class, but programmers might
> find
>> that weird if it's not where the method is actually defined. If they
> instead
>> try to name the defining class, the workaround is making the code more
> brittle.
>
> Yes indeed: I named the *current* class instead of the defining one
> intentionally in the fixit, because that would make the code behaves like
> if it wasn't qualify, if one day the method gets reimplemented in the
> current class.

Good.

> If the developer decides qualify the call using the defining class anyway,
> at least it will be clear what implementation will get called (but agreed
> this would make the code more brittle).
>
> 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.