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

John McCall via cfe-dev cfe-dev at lists.llvm.org
Mon Jan 14 11:31:51 PST 2019


On 14 Jan 2019, at 14:23, Aaron Ballman wrote:
> 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.

See my proposal earlier in the thread for how we can evolve the set of
"always"-on warnings more aggressively without breaking source compatibility.

John.

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


More information about the cfe-dev mailing list