r281198 - Add virtual destructor (necessary due to the switch to shared_ptr).

Richard Smith via cfe-commits cfe-commits at lists.llvm.org
Mon Sep 12 10:57:52 PDT 2016


On Mon, Sep 12, 2016 at 10:55 AM, David Blaikie via cfe-commits <
cfe-commits at lists.llvm.org> wrote:

> Does that actually need a virtual dtor? The type erasure handling in
> shared_ptr would handle it so long as each instance is constructed with the
> derived type (looks like it probably is - make_shared used consistently) &
> we should get a warning (or could make the base dtor protected non-virtual
> to ensure an error) if we miss that?
>

We get warnings from GCC regardless, and that broke a few bots :( I'd like
to switch this back to unique_ptr at some point (once I've figured out why
MSVC rejected it) and at that point we'll need the virtual destructor.

But I can understand that it's perhaps nice to do even if we could thread
> that delicate needle. Just curious.
>
> On Sun, Sep 11, 2016 at 11:59 PM Richard Smith via cfe-commits <
> cfe-commits at lists.llvm.org> wrote:
>
>> Author: rsmith
>> Date: Mon Sep 12 01:51:11 2016
>> New Revision: 281198
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=281198&view=rev
>> Log:
>> Add virtual destructor (necessary due to the switch to shared_ptr).
>>
>> Modified:
>>     cfe/trunk/utils/TableGen/ClangDiagnosticsEmitter.cpp
>>
>> Modified: cfe/trunk/utils/TableGen/ClangDiagnosticsEmitter.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/TableGen/
>> ClangDiagnosticsEmitter.cpp?rev=281198&r1=281197&r2=281198&view=diff
>> ============================================================
>> ==================
>> --- cfe/trunk/utils/TableGen/ClangDiagnosticsEmitter.cpp (original)
>> +++ cfe/trunk/utils/TableGen/ClangDiagnosticsEmitter.cpp Mon Sep 12
>> 01:51:11 2016
>> @@ -911,6 +911,7 @@ namespace {
>>  struct DiagText {
>>    struct Piece {
>>      virtual void print(std::vector<std::string> &RST) = 0;
>> +    virtual ~Piece() {}
>>    };
>>    struct TextPiece : Piece {
>>      StringRef Role;
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160912/30ed1010/attachment.html>


More information about the cfe-commits mailing list