On Mar 4, 2014, at 11:43 PM, Richard Smith <richard at metafoo.co.uk> wrote:> On Tue, Mar 4, 2014 at 10:59 PM, Chris Lattner <clattner at apple.com> wrote: > On Mar 4, 2014, at 10:48 PM, Duncan Exon Smith <dexonsmith at apple.com> wrote: >>>>> >>>>>> Related, should we require use of 'override' when methods override a base >>>>>> class method? >>>>> >>>>> My vote: require override. >>>> >>>> +1: override is useful and prevents errors. >>> Would it be too much to have clang emit a warning/error if override is missing? I know that sounds crazy and people hate errors which fire too often, but there’s not too much C++11 code out there yet, and so we have a chance to put errors/warnings in now without too much pain. People might just get used to them and think its how code has to be written :) >> >> Might be a nightmare when including legacy headers, but warnings can always be disabled... > > A clang warning for this would be awesome, but it should be off by default. That said, the build of LLVM itself could detect that Clang had this warning and turn it on. I think it would be great to have the makefiles/cmake detect modern clang's and turn on additional warnings that we can't inflict on the world by default. > > It might be reasonable to warn if a class has both a function marked 'override' and a function that overrides but is not marked 'override'.That could be useful - because it means that the author of the class is at least thinking about override - but having a "coding style" warning of "I always intend to use override" would still be useful. -Chris -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140305/6212877c/attachment.html>
On Wed, Mar 5, 2014 at 1:10 AM, Chris Lattner <clattner at apple.com> wrote:> > On Mar 4, 2014, at 11:43 PM, Richard Smith <richard at metafoo.co.uk> wrote: > > On Tue, Mar 4, 2014 at 10:59 PM, Chris Lattner <clattner at apple.com> wrote: >> >> On Mar 4, 2014, at 10:48 PM, Duncan Exon Smith <dexonsmith at apple.com> >> wrote: >> >> >> Related, should we require use of 'override' when methods override a base >> class method? >> >> >> My vote: require override. >> >> >> +1: override is useful and prevents errors. >> >> Would it be too much to have clang emit a warning/error if override is >> missing? I know that sounds crazy and people hate errors which fire too >> often, but there's not too much C++11 code out there yet, and so we have a >> chance to put errors/warnings in now without too much pain. People might >> just get used to them and think its how code has to be written :) >> >> >> Might be a nightmare when including legacy headers, but warnings can >> always be disabled... >> >> >> A clang warning for this would be awesome, but it should be off by >> default. That said, the build of LLVM itself could detect that Clang had >> this warning and turn it on. I think it would be great to have the >> makefiles/cmake detect modern clang's and turn on additional warnings that >> we can't inflict on the world by default. > > > It might be reasonable to warn if a class has both a function marked > 'override' and a function that overrides but is not marked 'override'. > > > That could be useful - because it means that the author of the class is at > least thinking about override - but having a "coding style" warning of "I > always intend to use override" would still be useful.Doug (not sure about other Clang owners) is pretty hesitant about implementing coding style warnings - anything with such a high false positive rate as to be off by default is assumed to be a non-starter in Clang (though perhaps things have changed in the years since I last tested the waters here). And now that we have something like clang-tidy, it's perhaps less of an issue... we'll see. - David
On Mar 5, 2014, at 9:53 AM, David Blaikie <dblaikie at gmail.com> wrote:>> It might be reasonable to warn if a class has both a function marked >> 'override' and a function that overrides but is not marked 'override'. >> >> >> That could be useful - because it means that the author of the class is at >> least thinking about override - but having a "coding style" warning of "I >> always intend to use override" would still be useful. > > Doug (not sure about other Clang owners) is pretty hesitant about > implementing coding style warnings - anything with such a high false > positive rate as to be off by default is assumed to be a non-starter > in Clang (though perhaps things have changed in the years since I last > tested the waters here). > > And now that we have something like clang-tidy, it's perhaps less of > an issue... we'll see.Making it part of clang-tidy would make a lot of sense then! Is there any plans to get clang-tidy running against the llvm/clang codebases regularly, or is it already happening? -Chris -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140305/ae43e457/attachment.html>