> On Sep 24, 2014, at 4:54 PM, David Blaikie <[email protected]> wrote:
> 
> 
> 
> On Wed, Sep 24, 2014 at 4:42 PM, Argyrios Kyrtzidis <[email protected] 
> <mailto:[email protected]>> 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 <[email protected] 
>> <mailto:[email protected]>> wrote:
>> 
>> 
>> 
>> On Wed, Sep 24, 2014 at 3:57 PM, jahanian <[email protected] 
>> <mailto:[email protected]>> 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
>> [email protected] <mailto:[email protected]>
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits 
>> <http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits>
>> 
>> 
>> _______________________________________________
>> cfe-commits mailing list
>> [email protected] <mailto:[email protected]>
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits 
>> <http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits>
> 
> 

_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to