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

[David Blaikie via cfe-commits]

Ah, hmm - we do use this technique in a few places. Oh, right - protected
in the base, final derived classes (maybe that's the missing element?). So
I think we've disabled whatever GCC warnings get in the way of that
particular cliche.

But, fair enough - thanks for the context. All power to the MSVC-compatible
engines.

On Mon, Sep 12, 2016 at 10:57 AM Richard Smith 
wrote:

> On Mon, Sep 12, 2016 at 10:55 AM, David Blaikie via cfe-commits <
> cfe-commits@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@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=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=281197=281198=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 ) = 0;
> +virtual ~Piece() {}
>};
>struct TextPiece : Piece {
>  StringRef Role;
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Richard Smith via cfe-commits]

On Mon, Sep 12, 2016 at 10:55 AM, David Blaikie via cfe-commits <
cfe-commits@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@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=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=281197=281198=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 ) = 0;
>> +virtual ~Piece() {}
>>};
>>struct TextPiece : Piece {
>>  StringRef Role;
>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[David Blaikie via cfe-commits]

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?

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@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=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=281197=281198=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 ) = 0;
> +virtual ~Piece() {}
>};
>struct TextPiece : Piece {
>  StringRef Role;
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits