On Thu, Mar 6, 2014 at 3:47 PM, Craig Topper <craig.topper at gmail.com> wrote:> virtual bar *foo() = 0; > > where foo() also exists as pure in the base class. Clang-modernize has a > FIXME that says it can't find the "=0" to do the insert of override. >Does that mean we have a pure virtual function with implementation in Clang/LLVM? If so, I feel it's a little bit confusing. If not, we should remove such redundant pure virtual function declarations from derived classes, no? On Thu, Mar 6, 2014 at 3:00 PM, Rui Ueyama <ruiu at google.com> wrote:> >> On Thu, Mar 6, 2014 at 2:21 PM, Craig Topper <craig.topper at gmail.com>wrote: >> >>> It also doesn't do pure methods either. >> >> >> I think I don't quite understand what that means. Can you give me an >> example? >> >> >>> On Thursday, March 6, 2014, Rui Ueyama <ruiu at google.com> wrote: >>> >>>> After running the tool aginst LLD, I realized that clang-modernize do >>>> not add "override" to virtual destructors. I think it's not intended but >>>> just that that case is not covered by the tool. >>>> >>>> >>>> On Wed, Mar 5, 2014 at 2:54 PM, Craig Topper <craig.topper at gmail.com>wrote: >>>> >>>> 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 <blangmuir at apple.com>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 <craig.topper at gmail.com> >>>> 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 <ruiu at google.com> wrote: >>>> >>>> On Wed, Mar 5, 2014 at 1:02 PM, Marshall Clow <mclow.lists at gmail.com>wrote: >>>> >>>> >>>> On Mar 5, 2014, at 10:29 AM, Chris Lattner <clattner at apple.com> wrote: >>>> >>>> >>>> On Mar 5, 2014, at 9:53 AM, David Blaikie <dblaikie at gmail.com> 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 >>>> >>>> >>> >>> -- >>> ~Craig >>> >> >> > > > -- > ~Craig >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140306/8b49e987/attachment.html>
The cases I saw were changing the return types. For example: TerminatorInst *clone_impl() const override = 0; this overrides virtual Instruction *clone_impl() const = 0; On Thu, Mar 6, 2014 at 4:23 PM, Rui Ueyama <ruiu at google.com> wrote:> On Thu, Mar 6, 2014 at 3:47 PM, Craig Topper <craig.topper at gmail.com>wrote: > >> virtual bar *foo() = 0; >> >> where foo() also exists as pure in the base class. Clang-modernize has a >> FIXME that says it can't find the "=0" to do the insert of override. >> > > Does that mean we have a pure virtual function with implementation in > Clang/LLVM? If so, I feel it's a little bit confusing. If not, we should > remove such redundant pure virtual function declarations from derived > classes, no? > > On Thu, Mar 6, 2014 at 3:00 PM, Rui Ueyama <ruiu at google.com> wrote: >> >>> On Thu, Mar 6, 2014 at 2:21 PM, Craig Topper <craig.topper at gmail.com>wrote: >>> >>>> It also doesn't do pure methods either. >>> >>> >>> I think I don't quite understand what that means. Can you give me an >>> example? >>> >>> >>>> On Thursday, March 6, 2014, Rui Ueyama <ruiu at google.com> wrote: >>>> >>>>> After running the tool aginst LLD, I realized that clang-modernize do >>>>> not add "override" to virtual destructors. I think it's not intended but >>>>> just that that case is not covered by the tool. >>>>> >>>>> >>>>> On Wed, Mar 5, 2014 at 2:54 PM, Craig Topper <craig.topper at gmail.com>wrote: >>>>> >>>>> 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 <blangmuir at apple.com>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 <craig.topper at gmail.com> >>>>> 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 <ruiu at google.com> wrote: >>>>> >>>>> On Wed, Mar 5, 2014 at 1:02 PM, Marshall Clow <mclow.lists at gmail.com>wrote: >>>>> >>>>> >>>>> On Mar 5, 2014, at 10:29 AM, Chris Lattner <clattner at apple.com> wrote: >>>>> >>>>> >>>>> On Mar 5, 2014, at 9:53 AM, David Blaikie <dblaikie at gmail.com> 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 >>>>> >>>>> >>>> >>>> -- >>>> ~Craig >>>> >>> >>> >> >> >> -- >> ~Craig >> > >-- ~Craig -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20140306/8a0bd6dd/attachment.html>
Looking a little closer StreamableMemoryObject.h does have some seemingly
redundant overrides.
And this override of a non-pure virtual with a pure virtual
virtual int readBytes(uint64_t address,
uint64_t size,
uint8_t *buf) const override = 0;
On Thu, Mar 6, 2014 at 7:27 PM, Craig Topper <craig.topper at gmail.com>
wrote:
> The cases I saw were changing the return types. For example:
>
> TerminatorInst *clone_impl() const override = 0;
>
> this overrides
>
> virtual Instruction *clone_impl() const = 0;
>
>
> On Thu, Mar 6, 2014 at 4:23 PM, Rui Ueyama <ruiu at google.com>
wrote:
>
>> On Thu, Mar 6, 2014 at 3:47 PM, Craig Topper <craig.topper at
gmail.com>wrote:
>>
>>> virtual bar *foo() = 0;
>>>
>>> where foo() also exists as pure in the base class. Clang-modernize
has a
>>> FIXME that says it can't find the "=0" to do the
insert of override.
>>>
>>
>> Does that mean we have a pure virtual function with implementation in
>> Clang/LLVM? If so, I feel it's a little bit confusing. If not, we
should
>> remove such redundant pure virtual function declarations from derived
>> classes, no?
>>
>> On Thu, Mar 6, 2014 at 3:00 PM, Rui Ueyama <ruiu at google.com>
wrote:
>>>
>>>> On Thu, Mar 6, 2014 at 2:21 PM, Craig Topper <craig.topper
at gmail.com>wrote:
>>>>
>>>>> It also doesn't do pure methods either.
>>>>
>>>>
>>>> I think I don't quite understand what that means. Can you
give me an
>>>> example?
>>>>
>>>>
>>>>> On Thursday, March 6, 2014, Rui Ueyama <ruiu at
google.com> wrote:
>>>>>
>>>>>> After running the tool aginst LLD, I realized that
clang-modernize do
>>>>>> not add "override" to virtual destructors. I
think it's not intended but
>>>>>> just that that case is not covered by the tool.
>>>>>>
>>>>>>
>>>>>> On Wed, Mar 5, 2014 at 2:54 PM, Craig Topper
<craig.topper at gmail.com>wrote:
>>>>>>
>>>>>> 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
<blangmuir at apple.com>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
<craig.topper at gmail.com>
>>>>>> 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 <ruiu at
google.com> wrote:
>>>>>>
>>>>>> On Wed, Mar 5, 2014 at 1:02 PM, Marshall Clow
<mclow.lists at gmail.com>wrote:
>>>>>>
>>>>>>
>>>>>> On Mar 5, 2014, at 10:29 AM, Chris Lattner <clattner
at apple.com>
>>>>>> wrote:
>>>>>>
>>>>>>
>>>>>> On Mar 5, 2014, at 9:53 AM, David Blaikie <dblaikie
at gmail.com> 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
>>>>>>
>>>>>>
>>>>>
>>>>> --
>>>>> ~Craig
>>>>>
>>>>
>>>>
>>>
>>>
>>> --
>>> ~Craig
>>>
>>
>>
>
>
> --
> ~Craig
>
--
~Craig
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20140306/079241a9/attachment.html>