| Summary: | g++ does not diagnose when upcasting owning pointer (e.g. unique_ptr) with non-virtual destructor | ||
|---|---|---|---|
| Product: | gcc | Reporter: | Matthew Woehlke <mw_triad> |
| Component: | libstdc++ | Assignee: | Not yet assigned to anyone <unassigned> |
| Status: | NEW --- | ||
| Severity: | normal | CC: | daniel.kruegler, dimhen, thiago, webrown.cpp |
| Priority: | P3 | Keywords: | diagnostic |
| Version: | 4.8.3 | ||
| Target Milestone: | --- | ||
| See Also: | https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58876 | ||
| Host: | Target: | ||
| Build: | Known to work: | ||
| Known to fail: | Last reconfirmed: | 2021-08-31 00:00:00 | |
(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... Because it's not a bug. This is a totally valid scenario. It might be valid with a custom deleter, but the example shown has undefined behaviour. N.B. we definitely want -Wdelete-non-virtual-dtor not the less useful -Wnon-virtual-dtor (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. (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. See https://wg21.link/p2413 which proposes to make this conversion ill-formed. |
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.)