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

[Alexander Kornienko]

On Wed, Mar 11, 2015 at 8:34 PM, David Blaikie <dblaikie at gmail.com> wrote:

> 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.
>
>

After updating my Clang and making fresh build directories with cmake, I
now get the non-virtual-dtor warning. Seems good, removed the default
destructor.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150312/9a132384/attachment.html>