[llvm] r238197 - Fix warning introduced in r238190 about lack of virtual destructor in MCObjectFileInfo.

[Daniel Sanders]

> From: David Blaikie [mailto:dblaikie at gmail.com] 
> Sent: 29 May 2015 01:57
> To: Daniel Sanders
> Cc: llvm-commits at cs.uiuc.edu
> Subject: Re: [llvm] r238197 - Fix warning introduced in r238190 about lack of virtual destructor in MCObjectFileInfo.
>
>
>
> On Thu, May 28, 2015 at 12:44 PM, Daniel Sanders <Daniel.Sanders at imgtec.com> wrote:
> > > > Are these objects actually destroyed polymorphically? If not, we should make the dtor non-virtual
> > > > and protected and the derived classes should be final.
> > >
> > > Sorry for the delay in replying to this. I had to revert r238197 and r238190 and I'm under time pressure
> > > to fix and re-commit.
> >
> > Yes, they're destroyed via a TargetLoweringObjectFile pointer.
> >
> > Were we not doing that before? Or did we not have derived classes before? Curious/confused.
>
> MCObjectFileInfo didn't have a virtual destructor but didn't have virtual functions either. It's immediate
> subclass (TargetLoweringObjectFile) had a virtual destructor and virtual functions. My patch added a virtual
> function to MCObjectFileInfo which introduced the warning about having virtual functions without a virtual
> destructor.
>
> But if it didn't change ownership (were things owned by TargetLoweringObjectFile* and are now
> owned/destroyed by MCObjectFileInfo*?) then it shouldn't've needed a virtual dtor. Perhaps this base class's
> dtor should be protected, then TargetLoweringObjectFile's dtor should be virtual (as it already must've been,
> if that's the point of ownership?)?

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.

> If there's only one subclass, why does it need to be virtual in this base class? 

To be honest, the main reason for adding the virtual destructor was that the compiler was complaining about it not being virtual.