[clang-tools-extra] r231938 - [clang-tidy] Clean up misc-use-override warning. NFC

[David Blaikie]

On Wed, Mar 11, 2015 at 11:22 AM, Alexander Kornienko <alexfh at google.com>
wrote:

> On Wed, Mar 11, 2015 at 6:29 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>>
>>
>> On Wed, Mar 11, 2015 at 10:21 AM, Alexander Kornienko <alexfh at google.com>
>> wrote:
>>
>>> On Wed, Mar 11, 2015 at 5:57 PM, David Blaikie <dblaikie at gmail.com>
>>> wrote:
>>>
>>>>
>>>>
>>>> On Wed, Mar 11, 2015 at 9:47 AM, Alexander Kornienko <alexfh at google.com
>>>> > wrote:
>>>>
>>>>> Author: alexfh
>>>>> Date: Wed Mar 11 11:47:27 2015
>>>>> New Revision: 231938
>>>>>
>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=231938&view=rev
>>>>> Log:
>>>>> [clang-tidy] Clean up misc-use-override warning. NFC
>>>>>
>>>>> Modified:
>>>>>     clang-tools-extra/trunk/clang-tidy/ClangTidy.h
>>>>>
>>>>> Modified: clang-tools-extra/trunk/clang-tidy/ClangTidy.h
>>>>> URL:
>>>>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidy.h?rev=231938&r1=231937&r2=231938&view=diff
>>>>>
>>>>> ==============================================================================
>>>>> --- clang-tools-extra/trunk/clang-tidy/ClangTidy.h (original)
>>>>> +++ clang-tools-extra/trunk/clang-tidy/ClangTidy.h Wed Mar 11 11:47:27
>>>>> 2015
>>>>> @@ -113,7 +113,7 @@ public:
>>>>>      assert(!CheckName.empty());
>>>>>    }
>>>>>
>>>>> -  virtual ~ClangTidyCheck() {}
>>>>> +  ~ClangTidyCheck() override = default;
>>>>>
>>>>
>>>> Just remove this, then - since it's the implicit default anyway?
>>>>
>>>
>>> I didn't want to rely on the implementation (that the class derives from
>>> another class that has a virtual destructor). Or would I get a warning in
>>> case I removed the inheritance?
>>>
>>
>> If this type has virtual functions and a public non-virtual dtor and is
>> not itself final, you'd get a warning.
>>
>
> Looks like the warning is is not turned on by default. When I remove
> "virtual" from the destructor of ast_matchers::MatchFinder::MatchCallback
> and I remove the destructor from ClangTidyCheck, the code compiles without
> any warnings.
>

Hmm? With an recent Clang self-hosting build I get:

/usr/local/google/home/blaikie/dev/llvm/src/tools/clang/include/clang/ASTMatchers/ASTMatchFinder.h:93:5:
error: 'clang::ast_matchers::MatchFinder::MatchCallback' has virtual
functions but non-virtual destructor [-Werror,-Wnon-virtual-dtor]
    ~MatchCallback();
    ^


Which version of clang are you using to build?


>
> It seems that a default overridden destructor is a better protection
> against this kind of error.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150311/0b74613b/attachment.html>