[LLVMdev] [RFC] C++11: 'virtual' and 'override'

[Sean Silva]

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>