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

[David Blaikie]

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?)?

If there's only one subclass, why does it need to be virtual in this base
class?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150528/17533548/attachment.html>