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>
Duncan P. N. Exon Smith
2014-Mar-05 03:57 UTC
[LLVMdev] [RFC] C++11: 'virtual' and 'override'
On 2014 Mar 4, at 15:01, Sean Silva <chisophugis at gmail.com> wrote:> 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.+1: virtual doesn’t add anything if override is present.> (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.+1: override is useful and prevents errors.> > 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 > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
On Mar 4, 2014, at 7:57 PM, Duncan P. N. Exon Smith <dexonsmith at apple.com> wrote:> On 2014 Mar 4, at 15:01, Sean Silva <chisophugis at gmail.com> wrote: > >> 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. > > +1: virtual doesn’t add anything if override is present. > >> (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. > > +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 :)> >>> 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 >> >> _______________________________________________ >> LLVM Developers mailing list >> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev > > > _______________________________________________ > 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/6192e50d/attachment.html>