While doing the conversion of LLVM_OVERRIDE to 'override' last night, I noticed that the code base is rather inconsistent on whether the 'virtual' keyword is also used when 'override' is used. Should we have a coding standard for this? What's the preferred direction here? Seems not having 'virtual' is less overall text, but not sure how others feel. Related, should we require use of 'override' when methods override a base class method? I have clang-tidy checks for both though haven't implemented fixits for them yet. Thanks, ~Craig -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140302/7c168f52/attachment.html>
On Sun, Mar 2, 2014 at 8:19 PM, Craig Topper <craig.topper at gmail.com> wrote:> While doing the conversion of LLVM_OVERRIDE to 'override' last night, I > noticed that the code base is rather inconsistent on whether the 'virtual' > keyword is also used when 'override' is used. > > Should we have a coding standard for this? What's the preferred direction > here? Seems not having 'virtual' is less overall text, but not sure how > others feel.My vote: omit virtual if override is used. (legitimate counterargument: harder to skim/match/read whether a function is virtual when it's not specified and "override" appears much later in the declaration)> Related, should we require use of 'override' when methods override a base > class method?My vote: require override.> I have clang-tidy checks for both though haven't implemented fixits for them > yet.Cool. There has also been a suggestion that Clang could warn about omitted override if at least one other member function in the same class is marked override, which could get us a lot of value built into the compiler (but not 'all the way', so a clang-tidy check would still be appropriate).
Hi folks! > [...] whether the 'virtual' keyword is also used when 'override' > is used. > Related, should we require use of 'override' when methods override > a base class method? I vote to require "override" and to leave out "virtual". By the way: This is what Delphi / Free Pascal do, The Delphi interpretation of "virtual" is to establish a new "slot" (i.e. a new entry in the VMT) whereas "override" re-uses such a slot. Jasper
On Mon, Mar 3, 2014 at 2:02 AM, David Blaikie <dblaikie at gmail.com> wrote:> On Sun, Mar 2, 2014 at 8:19 PM, Craig Topper <craig.topper at gmail.com> > wrote: > > While doing the conversion of LLVM_OVERRIDE to 'override' last night, I > > noticed that the code base is rather inconsistent on whether the > 'virtual' > > keyword is also used when 'override' is used. > > > > Should we have a coding standard for this? What's the preferred direction > > here? Seems not having 'virtual' is less overall text, but not sure how > > others feel. > > My vote: omit virtual if override is used. > > (legitimate counterargument: harder to skim/match/read whether a > function is virtual when it's not specified and "override" appears > much later in the declaration) >One counter-datapoint: Personally, I have on at least one occasion caught myself not noticing a leading `virtual` and thinking that a method wasn't overriden because of the missing `override`. I guess the moral is that this can be pretty adaptable. FWIW IMO the preferred end state is to have no useless leading `virtual`'s and using `override` for its intended purpose. -- Sean Silva> > > Related, should we require use of 'override' when methods override a base > > class method? > > My vote: require override. > > > I have clang-tidy checks for both though haven't implemented fixits for > them > > yet. > > Cool. There has also been a suggestion that Clang could warn about > omitted override if at least one other member function in the same > class is marked override, which could get us a lot of value built into > the compiler (but not 'all the way', so a clang-tidy check would still > be appropriate). > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140304/da9bf2c8/attachment.html>