[llvm] 4bb5f44 - [CMake] Remove unneeded -Wnon-virtual-dtor availability check

Martin Storsjö via llvm-commits llvm-commits at lists.llvm.org
Wed Sep 1 00:30:11 PDT 2021



> On Sep 1, 2021, at 10:21, Fangrui Song <i at maskray.me> wrote:
> 
> On Tue, Aug 31, 2021 at 11:58 PM Martin Storsjö <martin at martin.st> wrote:
>> 
>> This causes lots new build warnings when building with GCC (noted with GCC 7 and 9), like these:
>> 
>> ../include/llvm/Analysis/ScalarEvolution.h:200:7: warning: ‘class llvm::SCEVPredicate’ has virtual functions and accessible non-virtual destructor [-Wnon-virtual-dtor]
>> class SCEVPredicate : public FoldingSetNode {
>>       ^~~~~~~~~~~~~
>> ../include/llvm/Analysis/ScalarEvolution.h:267:7: warning: base class ‘class llvm::SCEVPredicate’ has accessible non-virtual destructor [-Wnon-virtual-dtor]
>> class SCEVEqualPredicate final : public SCEVPredicate {
>>       ^~~~~~~~~~~~~~~~~~
>> ../include/llvm/Analysis/ScalarEvolution.h:304:7: warning: base class ‘class llvm::SCEVPredicate’ has accessible non-virtual destructor [-Wnon-virtual-dtor]
>> class SCEVWrapPredicate final : public SCEVPredicate {
>>       ^~~~~~~~~~~~~~~~~
>> ../include/llvm/Analysis/ScalarEvolution.h:399:7: warning: base class ‘class llvm::SCEVPredicate’ has accessible non-virtual destructor [-Wnon-virtual-dtor]
>> class SCEVUnionPredicate final : public SCEVPredicate {
>>       ^~~~~~~~~~~~~~~~~~
>> 
>> I haven’t dug in to see whether this is the exact issue that the removed comment referred to or if it’s something else (or if the code should be fixed?).
>> 
>> // Martin
> 
> g++ 5.1, 7.5, and 9.5 -Wnon-virtual-dtor -std=c++14 do not warn on the
> tested code:
> 
> class base {public: virtual void anchor();protected: ~base();};
> class derived final : public base { public: ~derived();};
> int main() { return 0; }
> 
> Can you check -Wnon-virtual-dtor was previously disabled for GCC?
> 
> I haven't looked at include/llvm/Analysis/ScalarEvolution.h

Previously we only added -Wnon-virtual-dtor within the condition `if (NOT CMAKE_COMPILER_IS_GNUCXX AND NOT WIN32)`, so it was never added for GCC before afaik.

Ok, so then it sounds like the warning is potentially legit for those classes then?

// Martin




More information about the llvm-commits mailing list