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

[John McCall via cfe-dev]

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
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.
>>
>>