> On Oct 29, 2014, at 6:23 PM, Nico Weber <[email protected]> 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 _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
