<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Dec 3, 2015 at 9:00 AM, Andy Gibbs <span dir="ltr"><<a href="mailto:andyg1001@hotmail.co.uk" target="_blank">andyg1001@hotmail.co.uk</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On Thursday, December 03, 2015 5:29 PM, David Blaikie wrote:<span class=""><br>
On Thu, Dec 3, 2015 at 12:20 AM, Andy Gibbs wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Author: andyg<br>
Date: Thu Dec  3 02:20:20 2015<br>
New Revision: 254592<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=254592&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=254592&view=rev</a><br>
Log:<br>
Fix class SCEVPredicate has virtual functions and accessible non-virtual destructor.<br>
<br>
</blockquote>
<br>
Wait - why did the dtor need to be made virtual at all? These objects are never<br>
destroyed polymorphically, so far as I can tell.<br>
<br>
Could we just disable the GCC warning that was firing here instead? (if I<br>
understand correctly and this was a GCC warning)<br>
</blockquote>
<br></span>
Fine, but IMHO the warning is a valid one.  The code may not currently destroy these<br>
objects polymorphically, but if that changes in future (for whatever reason) then<br>
its a potential memory leak.  Its one of those "golden rule" moments, isn't it? :o)<br></blockquote><div><br></div><div>That's why the dtor is protected (& the derived classes with public non-virtual dtors are final). This pattern is used quite a bit in LLVM & the compiler diagnostics support it.<br><br>Both Clang and GCC don't warn in that case - but GCC does warn if that class then has any friends.<br><br>I don't see that having a friend or two should change the tradeoff here - you've exposed your internal details to those trusted pieces of code that, much liket he class itself, are restricted to not doing the bad thing.<br><br>Not quite sure what golden rule(s) you're referring to.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Rather than disable the warning, which could open up potential problems elsewhere<br>
not even related to this class, </blockquote><div><br></div><div>Except that it's highly restricted because the dtor is protected and the derived classes are final.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">I think it is better to fix the issue, even if it is<br>
technically/arguably a non-issue in this case.<br>
<br>
Its not like there is a huge overhead (code size, execution time, ...) to the fix.<br>
The classes already have vtables due to the other virtual methods, for example.<br></blockquote><div><br></div><div>But we shouldn't be adding features just because of a compiler warning - we should add features if/when we need them.<br><br>- Dave</div></div></div></div>