[Bug libstdc++/64399] New: g++ does not diagnose when upcasting owning pointer (e.g. unique_ptr) with non-virtual destructor

[Bug libstdc++/64399] New: g++ does not diagnose when upcasting owning pointer (e.g. unique_ptr) with non-virtual destructor

[mw_triad at users dot sourceforge.net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64399

            Bug ID: 64399
           Summary: g++ does not diagnose when upcasting owning pointer
                    (e.g. unique_ptr) with non-virtual destructor
           Product: gcc
           Version: 4.8.3
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: mw_triad at users dot sourceforge.net

Consider the following code:

  #include <memory>

  struct X { ~X(); };
  struct Y : X { ~Y(); };

  std::unique_ptr<Y> f();
  void g()
  {
    std::unique_ptr<X> x = f();
  }

This code is almost certainly broken; ownership of the constructed Y is
transferred to an owning unique_ptr in a way that will most likely (in general;
"always" in this specific example) result in Y's destructor never being called.

It seems pretty silly that there is no diagnostic for this. There ought to be a
warning in unique_ptr's conversion operator converting from Derived to Base if
~Base is not virtual. (Arranging at the library level to trip
-Wnon-virtual-dtor seems logical.)

[Bug libstdc++/64399] g++ does not diagnose when upcasting owning pointer (e.g. unique_ptr) with non-virtual destructor

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64399

--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> ---
PR 58876

[Bug libstdc++/64399] g++ does not diagnose when upcasting owning pointer (e.g. unique_ptr) with non-virtual destructor

[mw_triad at users dot sourceforge.net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64399

--- Comment #2 from Matthew Woehlke <mw_triad at users dot sourceforge.net> ---
(In reply to Jonathan Wakely from comment #1)
> PR 58876

*Almost*, except I am proposing that -Wnon-virtual-dtor should trip even if X
does not otherwise have virtual methods. (Just why you'd be writing such code,
I'm not sure, but...)

Odd that didn't turn up in my search for existing bugs...

[Bug libstdc++/64399] g++ does not diagnose when upcasting owning pointer (e.g. unique_ptr) with non-virtual destructor

[thiago at kde dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64399

Thiago Macieira <thiago at kde dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |thiago at kde dot org

--- Comment #3 from Thiago Macieira <thiago at kde dot org> ---
Because it's not a bug.

This is a totally valid scenario.

[Bug libstdc++/64399] g++ does not diagnose when upcasting owning pointer (e.g. unique_ptr) with non-virtual destructor

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64399

--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> ---
It might be valid with a custom deleter, but the example shown has undefined
behaviour.

[Bug libstdc++/64399] g++ does not diagnose when upcasting owning pointer (e.g. unique_ptr) with non-virtual destructor

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64399

--- Comment #5 from Jonathan Wakely <redi at gcc dot gnu.org> ---
N.B. we definitely want -Wdelete-non-virtual-dtor not the less useful
-Wnon-virtual-dtor

[Bug libstdc++/64399] g++ does not diagnose when upcasting owning pointer (e.g. unique_ptr) with non-virtual destructor

[glisse at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64399

--- Comment #6 from Marc Glisse <glisse at gcc dot gnu.org> ---
(In reply to Jonathan Wakely from comment #4)
> It might be valid with a custom deleter, but the example shown has undefined
> behaviour.

When the derived class does not add any member or redefine any important
functionality, it is not an uncommon technique to call the base class
destructor on a derived class. It might pedantically be illegal, but it is
useful, and I believe some people would like to avoid the warning when the two
destructors are equivalent.

[Bug libstdc++/64399] g++ does not diagnose when upcasting owning pointer (e.g. unique_ptr) with non-virtual destructor

[mw_triad at users dot sourceforge.net]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64399

--- Comment #7 from Matthew Woehlke <mw_triad at users dot sourceforge.net> ---
(In reply to Thiago Macieira from comment #3)
> Because it's not a bug.
> 
> This is a totally valid scenario.

Valid in what way? I constructed a Y but arranged, probably by accident, that
its dtor is never called. I fail to see how that's not likely a bug in my code
that reasonably warrants a diagnostic. (Note that I'm talking about a
*warning*, and possibly one that isn't even on by default, not an error.)

(In reply to Marc Glisse from comment #6)
> It might pedantically be illegal, but it is useful, and I believe some people
> would like to avoid the warning when the two destructors are equivalent.

However, the compiler doesn't know that here, because I didn't provided a
definition thereof; Y's dtor, even in this example, could have important side
effects. Even if the compiler *can* prove equivalence, I'd be suspicious
whether this was intended, but I'd be okay with a different (i.e. more
pedantic) warning in that case. (I'd also point out that it's not unreasonable
to require the user to somehow annotate if this is intentional if they care
about avoiding the warning when it's enabled.)

Anyway, I still get no warning if Y has members that need to be destroyed,
which definitely causes bad behavior when its dtor isn't called.

[Bug libstdc++/64399] g++ does not diagnose when upcasting owning pointer (e.g. unique_ptr) with non-virtual destructor

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64399

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |diagnostic
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2021-08-31
     Ever confirmed|0                           |1

--- Comment #8 from Jonathan Wakely <redi at gcc dot gnu.org> ---
See https://wg21.link/p2413 which proposes to make this conversion ill-formed.