<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><br><div><div>On Mar 4, 2014, at 7:57 PM, Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com">dexonsmith@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div style="font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;">On 2014 Mar 4, at 15:01, Sean Silva <<a href="mailto:chisophugis@gmail.com">chisophugis@gmail.com</a>> wrote:<br><br><blockquote type="cite">On Mon, Mar 3, 2014 at 2:02 AM, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>On Sun, Mar 2, 2014 at 8:19 PM, Craig Topper <<a href="mailto:craig.topper@gmail.com">craig.topper@gmail.com</a>> wrote:<br><blockquote type="cite">While doing the conversion of LLVM_OVERRIDE to 'override' last night, I<br>noticed that the code base is rather inconsistent on whether the 'virtual'<br>keyword is also used when 'override' is used.<br><br>Should we have a coding standard for this? What's the preferred direction<br>here? Seems not having 'virtual' is less overall text, but not sure how<br>others feel.<br></blockquote><br>My vote: omit virtual if override is used.<br></blockquote><br>+1: virtual doesn’t add anything if override is present.<br><br><blockquote type="cite">(legitimate counterargument: harder to skim/match/read whether a<br>function is virtual when it's not specified and "override" appears<br>much later in the declaration)<br><br>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.<br><br>FWIW IMO the preferred end state is to have no useless leading `virtual`'s and using `override` for its intended purpose.<br><br>-- Sean Silva<br><br><br><blockquote type="cite">Related, should we require use of 'override' when methods override a base<br>class method?<br></blockquote><br>My vote: require override.<br></blockquote><br>+1: override is useful and prevents errors.<br></div></blockquote>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 :)<br><blockquote type="cite"><div style="font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;"><br><blockquote type="cite"><blockquote type="cite">I have clang-tidy checks for both though haven't implemented fixits for them<br>yet.<br></blockquote><br>Cool. There has also been a suggestion that Clang could warn about<br>omitted override if at least one other member function in the same<br>class is marked override, which could get us a lot of value built into<br>the compiler (but not 'all the way', so a clang-tidy check would still<br>be appropriate).<br>_______________________________________________<br>LLVM Developers mailing list<br><a href="mailto:LLVMdev@cs.uiuc.edu">LLVMdev@cs.uiuc.edu</a> <a href="http://llvm.cs.uiuc.edu">http://llvm.cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev">http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev</a><br><br>_______________________________________________<br>LLVM Developers mailing list<br>LLVMdev@cs.uiuc.edu http://llvm.cs.uiuc.edu<br>http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev<br></blockquote><br><br>_______________________________________________<br>LLVM Developers mailing list<br><a href="mailto:LLVMdev@cs.uiuc.edu">LLVMdev@cs.uiuc.edu</a><span class="Apple-converted-space"> </span> <a href="http://llvm.cs.uiuc.edu/">http://llvm.cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev">http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev</a></div></blockquote></div><br></body></html>