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

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
27 messages Options
12
Reply | Threaded
Open this post in threaded view
|

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

Craig Topper
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

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

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

David Blaikie
On Sun, Mar 2, 2014 at 8:19 PM, Craig Topper <[hidden email]> 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).
_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

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

Jasper Neumann-2
In reply to this post by Craig Topper
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
_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

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

Sean Silva-2
In reply to this post by David Blaikie



On Mon, Mar 3, 2014 at 2:02 AM, David Blaikie <[hidden email]> wrote:
On Sun, Mar 2, 2014 at 8:19 PM, Craig Topper <[hidden email]> 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
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev


_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

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

Duncan P. N. Exon Smith
On 2014 Mar 4, at 15:01, Sean Silva <[hidden email]> wrote:

> On Mon, Mar 3, 2014 at 2:02 AM, David Blaikie <[hidden email]> wrote:
> On Sun, Mar 2, 2014 at 8:19 PM, Craig Topper <[hidden email]> 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
> [hidden email]         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>
> _______________________________________________
> LLVM Developers mailing list
> [hidden email]         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev


_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

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

Pete Cooper

On Mar 4, 2014, at 7:57 PM, Duncan P. N. Exon Smith <[hidden email]> wrote:

On 2014 Mar 4, at 15:01, Sean Silva <[hidden email]> wrote:

On Mon, Mar 3, 2014 at 2:02 AM, David Blaikie <[hidden email]> wrote:
On Sun, Mar 2, 2014 at 8:19 PM, Craig Topper <[hidden email]> 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
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev


_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev


_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

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

Chandler Carruth-2

On Tue, Mar 4, 2014 at 10:38 PM, Pete Cooper <[hidden email]> wrote:
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 :)

There is already work on a clang-tidy check for this.

We actually have the framework for Chris's asked-for coding convention checker. Now we get to use it. =D

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

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

Duncan P. N. Exon Smith
In reply to this post by Pete Cooper

On Mar 4, 2014, at 22:38, Pete Cooper <[hidden email]> wrote:


On Mar 4, 2014, at 7:57 PM, Duncan P. N. Exon Smith <[hidden email]> wrote:

On 2014 Mar 4, at 15:01, Sean Silva <[hidden email]> wrote:

On Mon, Mar 3, 2014 at 2:02 AM, David Blaikie <[hidden email]> wrote:
On Sun, Mar 2, 2014 at 8:19 PM, Craig Topper <[hidden email]> 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 :)

Might be a nightmare when including legacy headers, but warnings can always be disabled...


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
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev


_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev


_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

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

Chris Lattner-2
On Mar 4, 2014, at 10:48 PM, Duncan Exon Smith <[hidden email]> 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.

-Chris

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

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

Richard Smith-33
On Tue, Mar 4, 2014 at 10:59 PM, Chris Lattner <[hidden email]> wrote:
On Mar 4, 2014, at 10:48 PM, Duncan Exon Smith <[hidden email]> 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'.

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

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

Chris Lattner-2

On Mar 4, 2014, at 11:43 PM, Richard Smith <[hidden email]> wrote:

On Tue, Mar 4, 2014 at 10:59 PM, Chris Lattner <[hidden email]> wrote:
On Mar 4, 2014, at 10:48 PM, Duncan Exon Smith <[hidden email]> 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

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

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

Joerg Sonnenberger
In reply to this post by Duncan P. N. Exon Smith
On Tue, Mar 04, 2014 at 10:48:49PM -0800, Duncan Exon Smith wrote:
> Might be a nightmare when including legacy headers, but warnings can always be disabled...

What about tagging the class as requiring override? Could be an
attribute or pragma...

Joerg
_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

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

David Blaikie
In reply to this post by Chris Lattner-2
On Wed, Mar 5, 2014 at 1:10 AM, Chris Lattner <[hidden email]> wrote:

>
> On Mar 4, 2014, at 11:43 PM, Richard Smith <[hidden email]> wrote:
>
> On Tue, Mar 4, 2014 at 10:59 PM, Chris Lattner <[hidden email]> wrote:
>>
>> On Mar 4, 2014, at 10:48 PM, Duncan Exon Smith <[hidden email]>
>> 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
_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

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

Chris Lattner-2

On Mar 5, 2014, at 9:53 AM, David Blaikie <[hidden email]> 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

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

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

Chandler Carruth-2

On Wed, Mar 5, 2014 at 10:29 AM, Chris Lattner <[hidden email]> wrote:
On Mar 5, 2014, at 9:53 AM, David Blaikie <[hidden email]> 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?

Alex and others here are actively working on it. I think Craig Topper has even used a prototype tidy check for this exact issue to do most of the cleanup. =]

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

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

Marshall Clow-2
In reply to this post by Chris Lattner-2

On Mar 5, 2014, at 10:29 AM, Chris Lattner <[hidden email]> wrote:


On Mar 5, 2014, at 9:53 AM, David Blaikie <[hidden email]> 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?

I believe that the “clang-modernize” tool can add “override” in the appropriate places.

— Marshall



_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

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

Rui Ueyama
On Wed, Mar 5, 2014 at 1:02 PM, Marshall Clow <[hidden email]> wrote:

On Mar 5, 2014, at 10:29 AM, Chris Lattner <[hidden email]> wrote:


On Mar 5, 2014, at 9:53 AM, David Blaikie <[hidden email]> 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?

I believe that the “clang-modernize” tool can add “override” in the appropriate places.

Can it also delete "virtual" if it has "override"?
 

— Marshall



_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev



_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

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

Craig Topper
clang-modernize can add the 'override', but it can't currently delete 'virtual'. It will also potentially overflow 80 columns. And if it removed virtual it would fail to align a second line of arguments correctly. So you need modernize and clang-format I guess. Though I'm not sure we want to widespread apply clang-format.


On Wed, Mar 5, 2014 at 1:10 PM, Rui Ueyama <[hidden email]> wrote:
On Wed, Mar 5, 2014 at 1:02 PM, Marshall Clow <[hidden email]> wrote:

On Mar 5, 2014, at 10:29 AM, Chris Lattner <[hidden email]> wrote:


On Mar 5, 2014, at 9:53 AM, David Blaikie <[hidden email]> 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?

I believe that the “clang-modernize” tool can add “override” in the appropriate places.

Can it also delete "virtual" if it has "override"?
 

— Marshall



_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev



_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev




--
~Craig

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

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

Ben Langmuir
clang-modernize has a -format option that will run clang-format on the code it changes.

Ben


On Mar 5, 2014, at 2:26 PM, Craig Topper <[hidden email]> wrote:

clang-modernize can add the 'override', but it can't currently delete 'virtual'. It will also potentially overflow 80 columns. And if it removed virtual it would fail to align a second line of arguments correctly. So you need modernize and clang-format I guess. Though I'm not sure we want to widespread apply clang-format.


On Wed, Mar 5, 2014 at 1:10 PM, Rui Ueyama <[hidden email]> wrote:
On Wed, Mar 5, 2014 at 1:02 PM, Marshall Clow <[hidden email]> wrote:

On Mar 5, 2014, at 10:29 AM, Chris Lattner <[hidden email]> wrote:


On Mar 5, 2014, at 9:53 AM, David Blaikie <[hidden email]> 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?

I believe that the “clang-modernize” tool can add “override” in the appropriate places.

Can it also delete "virtual" if it has "override"?
 

— Marshall



_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev



_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev




--
~Craig
_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev


_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
Reply | Threaded
Open this post in threaded view
|

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

Craig Topper
Didn't realize that. I'll see if i can figure out how to make it delete the virtual keyword.


On Wed, Mar 5, 2014 at 2:32 PM, Ben Langmuir <[hidden email]> wrote:
clang-modernize has a -format option that will run clang-format on the code it changes.

Ben



On Mar 5, 2014, at 2:26 PM, Craig Topper <[hidden email]> wrote:

clang-modernize can add the 'override', but it can't currently delete 'virtual'. It will also potentially overflow 80 columns. And if it removed virtual it would fail to align a second line of arguments correctly. So you need modernize and clang-format I guess. Though I'm not sure we want to widespread apply clang-format.


On Wed, Mar 5, 2014 at 1:10 PM, Rui Ueyama <[hidden email]> wrote:
On Wed, Mar 5, 2014 at 1:02 PM, Marshall Clow <[hidden email]> wrote:

On Mar 5, 2014, at 10:29 AM, Chris Lattner <[hidden email]> wrote:


On Mar 5, 2014, at 9:53 AM, David Blaikie <[hidden email]> 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?

I believe that the “clang-modernize” tool can add “override” in the appropriate places.

Can it also delete "virtual" if it has "override"?
 

— Marshall



_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev



_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev




--
~Craig
_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev




--
~Craig

_______________________________________________
LLVM Developers mailing list
[hidden email]         http://llvm.cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
12