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

[David Blaikie]

On Wed, Mar 11, 2015 at 12:25 PM, Alexander Kornienko <alexfh at google.com>
wrote:

> On Wed, Mar 11, 2015 at 8:23 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>>
>>
>> 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?
>>
>
>
> clang version 3.5.0 (207890)
>

You need at least 208449 to pass the check in
cmake/modules/HandleLLVMOptions.cmake that checks that the compiler has
good support for -Wnon-virtual-dtor.


>
>
>>
>>>
>>> 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/42b15117/attachment.html>