<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, May 29, 2015 at 6:37 AM, Daniel Sanders <span dir="ltr"><<a href="mailto:Daniel.Sanders@imgtec.com" target="_blank">Daniel.Sanders@imgtec.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">> From: David Blaikie [mailto:<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>]<br>
</span>> Sent: 29 May 2015 01:57<br>
<span class="">> To: Daniel Sanders<br>
> Cc: <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
> Subject: Re: [llvm] r238197 - Fix warning introduced in r238190 about lack of virtual destructor in MCObjectFileInfo.<br>
><br>
><br>
><br>
</span><div><div class="h5">> On Thu, May 28, 2015 at 12:44 PM, Daniel Sanders <<a href="mailto:Daniel.Sanders@imgtec.com">Daniel.Sanders@imgtec.com</a>> wrote:<br>
> > > > Are these objects actually destroyed polymorphically? If not, we should make the dtor non-virtual<br>
> > > > and protected and the derived classes should be final.<br>
> > ><br>
> > > Sorry for the delay in replying to this. I had to revert r238197 and r238190 and I'm under time pressure<br>
> > > to fix and re-commit.<br>
> ><br>
> > Yes, they're destroyed via a TargetLoweringObjectFile pointer.<br>
> ><br>
> > Were we not doing that before? Or did we not have derived classes before? Curious/confused.<br>
><br>
> MCObjectFileInfo didn't have a virtual destructor but didn't have virtual functions either. It's immediate<br>
> subclass (TargetLoweringObjectFile) had a virtual destructor and virtual functions. My patch added a virtual<br>
> function to MCObjectFileInfo which introduced the warning about having virtual functions without a virtual<br>
> destructor.<br>
><br>
> But if it didn't change ownership (were things owned by TargetLoweringObjectFile* and are now<br>
> owned/destroyed by MCObjectFileInfo*?) then it shouldn't've needed a virtual dtor. Perhaps this base class's<br>
> dtor should be protected, then TargetLoweringObjectFile's dtor should be virtual (as it already must've been,<br>
> if that's the point of ownership?)?<br>
<br>
</div></div>I think I see how to choose between the two now. If the virtual function is still there when I try to re-commit then I'll use a protected non-virtual destructor.<br></blockquote><div><br>SGTM<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class=""><br>
> If there's only one subclass, why does it need to be virtual in this base class? <br>
<br>
</span>To be honest, the main reason for adding the virtual destructor was that the compiler was complaining about it not being virtual.<br>
</blockquote></div><br></div></div>