CXX11 patch to warn if 'override' is missing on overriding virtual function

[Douglas Gregor]

> On Sep 24, 2014, at 4:54 PM, David Blaikie <dblaikie at gmail.com> wrote:
> 
> 
> 
> On Wed, Sep 24, 2014 at 4:42 PM, Argyrios Kyrtzidis <kyrtzidis at apple.com <mailto:kyrtzidis at apple.com>> wrote:
> This isn’t a style checker, this is to make sure you are not overriding something by accident.
> 
> For context, I took those quotations from a thread about a warning for "using a null pointer other than nullptr" which seems pretty analogous to this warning. "Use this feature because it might help you find bugs" but I think Doug was basically saying that the guideline to use a particular feature is a stylistic one, even if that style then leads to finding bugs.

Yeah, both warnings fall into the “introduce extra syntax, the consistent use of which will prevent bugs” camp. One can choose to use the feature if the syntactic overhead is less burdensome than tracking down the bugs, or not. 

My position has softened somewhat, and I don’t feel as strongly that these warnings is “purely stylistic”. I still think an off-by-default warning is extremely unfortunate, because new warning flags aren’t that discoverable. I’d feel a lot better if some part of the warning could be on by default. For example, if you’ve uttered “override” at least once in a class, it makes sense to warn-by-default about any other overrides in that class that weren’t marked as “override”, because you’re being locally inconsistent. Or maybe you can expand that heuristic out to a file-level granularity (which matches better for the null point constant warning) and still be on-by-default. Explicitly providing the warning flag (or some other related warning flag) could turn the warning on everywhere, for those who want the rule consistently applied to their code base.

	- Doug

> 
> It needs to be off-by-default due to the vast amount of C++ code that exists out there, but we can have it enabled when you create a new project.
> 
>> On Sep 24, 2014, at 4:16 PM, David Blaikie <dblaikie at gmail.com <mailto:dblaikie at gmail.com>> wrote:
>> 
>> 
>> 
>> On Wed, Sep 24, 2014 at 3:57 PM, jahanian <fjahanian at apple.com <mailto:fjahanian at apple.com>> wrote:
>> Hi all,
>> 
>> We have an enhancement request  from our users to provide
>> warning if ‘override’ control is missing. This warning is off by default as it will
>> be noisy (but it will show itself with -Weverything).
>> Is such a patch useful enough to go into the trunk? Also, comment on the patch is appreciated.
>> I will provide ‘fixit’ later if this is a worthwhile patch.
>> 
>> While I rather like the idea of such a warning, the usual bar has been a strong aversion to adding off-by-default warnings. I think the theoretical future might be building warnings like this into clang-tidy, then providing some plugin-like option to enable certain clang-tidy warnings in your normal builds.
>> 
>> (because I was curious, I went back & found some choice quotes from Doug Gregor on warnings like this (this is what he told me, years ago, when I proposed adding a warning for null pointers that aren't nullptr*):
>> 
>> "Off-by-default warnings are not a mechanism to subvert our normal processes for vetting awarning. In general, we should avoid off-by-default warnings: if it's not good enough to turn on by default, why do we have it at all? The vast majority of users will never see an off-by-default warning."
>> "A compiler is not a style checker, nor should it ever be."
>> "Warnings are intended to find potential problems in the source code. Style migration is the domain of separate tools.")
>> 
>> (& cc'ing Doug in case there's something about this that's different/things have changed over the years)
>> 
>> * today, I'd probably be able to get that in on the basis of compatibility with GCC's -Wzero-as-null-pointer-constant
>>  
>> 
>> - Fariborz
>> 
>>         
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu <mailto:cfe-commits at cs.uiuc.edu>
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits <http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits>
>> 
>> 
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu <mailto:cfe-commits at cs.uiuc.edu>
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits <http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits>
> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140925/d97fb06f/attachment.html>