<div dir="ltr"><div dir="ltr">On Wed, Sep 1, 2021 at 12:30 AM Martin Storsjö via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br></div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>
<br>
> On Sep 1, 2021, at 10:21, Fangrui Song <<a href="mailto:i@maskray.me" target="_blank">i@maskray.me</a>> wrote:<br>
> <br>
> On Tue, Aug 31, 2021 at 11:58 PM Martin Storsjö <<a href="mailto:martin@martin.st" target="_blank">martin@martin.st</a>> wrote:<br>
>> <br>
>> This causes lots new build warnings when building with GCC (noted with GCC 7 and 9), like these:<br>
>> <br>
>> ../include/llvm/Analysis/ScalarEvolution.h:200:7: warning: ‘class llvm::SCEVPredicate’ has virtual functions and accessible non-virtual destructor [-Wnon-virtual-dtor]<br>
>> class SCEVPredicate : public FoldingSetNode {<br>
>>       ^~~~~~~~~~~~~<br>
>> ../include/llvm/Analysis/ScalarEvolution.h:267:7: warning: base class ‘class llvm::SCEVPredicate’ has accessible non-virtual destructor [-Wnon-virtual-dtor]<br>
>> class SCEVEqualPredicate final : public SCEVPredicate {<br>
>>       ^~~~~~~~~~~~~~~~~~<br>
>> ../include/llvm/Analysis/ScalarEvolution.h:304:7: warning: base class ‘class llvm::SCEVPredicate’ has accessible non-virtual destructor [-Wnon-virtual-dtor]<br>
>> class SCEVWrapPredicate final : public SCEVPredicate {<br>
>>       ^~~~~~~~~~~~~~~~~<br>
>> ../include/llvm/Analysis/ScalarEvolution.h:399:7: warning: base class ‘class llvm::SCEVPredicate’ has accessible non-virtual destructor [-Wnon-virtual-dtor]<br>
>> class SCEVUnionPredicate final : public SCEVPredicate {<br>
>>       ^~~~~~~~~~~~~~~~~~<br>
>> <br>
>> 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?).<br>
>> <br>
>> // Martin<br>
> <br>
> g++ 5.1, 7.5, and 9.5 -Wnon-virtual-dtor -std=c++14 do not warn on the<br>
> tested code:<br>
> <br>
> class base {public: virtual void anchor();protected: ~base();};<br>
> class derived final : public base { public: ~derived();};<br>
> int main() { return 0; }<br>
> <br>
> Can you check -Wnon-virtual-dtor was previously disabled for GCC?<br>
> <br>
> I haven't looked at include/llvm/Analysis/ScalarEvolution.h<br>
<br>
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.<br>
<br>
Ok, so then it sounds like the warning is potentially legit for those classes then?<br></blockquote><div><br>Based on a difference in behavior with the GCC warning (it warns on final classes/those with protected destructors (both Clang and GCC generally suppress the warning in this case) if the type has a friend - under the assumption that the friend might violate the constraints of the type and try to destroy it polymorphically - I think this is a bug/misfeature/not a constraint I'd want to enforce in LLVM's codebase)<br><br>Given the friend "bug"/implementation choice in GCC's version of this warning - I think we should rollback the fixes (since they aren't needed - they pessimize code unnecessarily) and disable this warning under gcc.<br><br>Looks like all these patches might've been unnecessary/workarounds for the GCC behavior:<br><a href="https://reviews.llvm.org/rG4852c770fe87">https://reviews.llvm.org/rG4852c770fe87</a><br><a href="https://github.com/llvm/llvm-project/commit/6df09d6ccbc0cb72d3278cafb592e9bc0e6b84a1">https://github.com/llvm/llvm-project/commit/6df09d6ccbc0cb72d3278cafb592e9bc0e6b84a1</a><br><a href="https://github.com/llvm/llvm-project/commit/4852c770fe8703145dd2a35395985646ce57a454">https://github.com/llvm/llvm-project/commit/4852c770fe8703145dd2a35395985646ce57a454</a><br></div></div></div>