[llvm] r174483 - Add virtual desctructor to FrameEntry to avoid error on delete-non-virtual-dtor

[David Blaikie]

On Wed, Feb 6, 2013 at 8:14 AM, Eli Bendersky <eliben at google.com> wrote:
> On Tue, Feb 5, 2013 at 7:57 PM, David Blaikie <dblaikie at gmail.com> wrote:
>>
>> On Feb 5, 2013 7:11 PM, "Eli Bendersky" <eliben at google.com> wrote:
>>>
>>> Author: eliben
>>> Date: Tue Feb  5 21:08:02 2013
>>> New Revision: 174483
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=174483&view=rev
>>> Log:
>>> Add virtual desctructor to FrameEntry to avoid error on
>>> delete-non-virtual-dtor
>>>
>>> Modified:
>>>     llvm/trunk/lib/DebugInfo/DWARFDebugFrame.cpp
>>>
>>> Modified: llvm/trunk/lib/DebugInfo/DWARFDebugFrame.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/DebugInfo/DWARFDebugFrame.cpp?rev=174483&r1=174482&r2=174483&view=diff
>>>
>>> ==============================================================================
>>> --- llvm/trunk/lib/DebugInfo/DWARFDebugFrame.cpp (original)
>>> +++ llvm/trunk/lib/DebugInfo/DWARFDebugFrame.cpp Tue Feb  5 21:08:02 2013
>>> @@ -23,6 +23,9 @@ public:
>>>    FrameEntry(FrameKind K, DataExtractor D, uint64_t Offset, uint64_t
>>> Length)
>>>      : Kind(K), Data(D), Offset(Offset), Length(Length) {}
>>>
>>> +  virtual ~FrameEntry() {
>>> +  }
>>> +
>>>    FrameKind getKind() const { return Kind; }
>>>
>>>    virtual void dumpHeader(raw_ostream &OS) const = 0;
>>> @@ -54,6 +57,9 @@ public:
>>>       DataAlignmentFactor(DataAlignmentFactor),
>>>       ReturnAddressRegister(ReturnAddressRegister) {}
>>>
>>> +  ~CIE() {
>>> +  }
>>
>> Why did you add these explicit destructors that are the same as the implicit
>> ones?
>
> Empty explicit desctructors are something I've seen in other places in
> LLVM code.

Have you seen them inline like that much?

Out of line empty explicit destructors are sometimes used as key
functions to anchor the vtable emission (
http://llvm.org/docs/CodingStandards.html#provide-a-virtual-method-anchor-for-classes-in-headers
).

> They're not semantically required, from a strict point of
> view. I don't think we have a specific style guideline about this, and
> don't feel strongly about either option.

I'd vote for leaving things out that are just a duplication of the
implicit functionality anyway. To argue from consistency I'd wager the
codebase has far more implicit default ctors than it has explicit
empty ones.