r220703 - c++11 patch to issue warning on missing 'override' on

jahanian fjahanian at apple.com
Thu Oct 30 09:21:31 PDT 2014


> On Oct 29, 2014, at 6:23 PM, Nico Weber <thakis at chromium.org> wrote:
> 
> I've now looked at a bit over 1000 instances of this warning (and fixed many of them); with this experience I have two more pieces of feedback:
> 
> 1. This is a very cool warning.
> 2. In practice, the methods this warns on sometimes comes from a macro and it's impossible to add override. Examples are gmock's MOCK_METHODn, and IFACEMETHOD and COM_INTERFACE_ENTRY from the Windows SDK. Maybe this shouldn't warn if the method it warns on comes from a macro expansion? Or, if you think that that would hide too many issues (it did find a few cases where I could just add "override" to a macro, in my experience), maybe at least if the macro that's expanded is from a system header?
> 
> Thanks,
> Nico
> 


To my (pleasant) surprise, there were not any builedbot issues of late. So, it seems that people are following the practice of adding ‘override’
before this patch hit (of course I made sure that llvm builds do not warn beforehand). Would it be hard to fix Windows SDK’s macros to add this keyword?
One practice has been to not warn if condition occurs in system headers (as you suggested). This is my preference should we decide to restrict this warning.
But since Richard has reviewed the patch, I would like to hear from him as well.
- Fariborz





More information about the cfe-commits mailing list