[Bug c++/102168] New: -Wnon-virtual-dtor shouldn't fire for protected dtor in a class with a friend declaration

[Bug c++/102168] New: -Wnon-virtual-dtor shouldn't fire for protected dtor in a class with a friend declaration

[i at maskray dot me]

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

            Bug ID: 102168
           Summary: -Wnon-virtual-dtor shouldn't fire for protected dtor
                    in a class with a friend declaration
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: i at maskray dot me
  Target Milestone: ---

class base;

class b {
public:
  void del(base *x);
};

class base {
  friend b;
public:
  virtual void anchor();
protected:
  virtual  // why is this needed?
  ~base() = default;
};

class derived final : public base { 
public: 
  ~derived() {}
};

void b::del(base *x) {
  delete x;
}

% g++ -c -Wnon-virtual-dtor a.cc
a.cc:8:7: warning: ‘class base’ has virtual functions and accessible
non-virtual destructor [-Wnon-virtual-dtor]
    8 | class base {
      |       ^~~~
a.cc:17:7: warning: base class ‘class base’ has accessible non-virtual
destructor [-Wnon-virtual-dtor]
   17 | class derived final : public base {
      |       ^~~~~~~


This diagnostic is due to a friend declaration because technically the friend
can invoke the dtor.

However, this seems a bit dumb (https://reviews.llvm.org/rG4852c770fe87)
It just checks the existence of friend, not actually checking whether the dtor
is actually used.
Checking whether the dtor is actually needed requires dataflow analysis (like
frontend devirtualization), which is apparently too heavy and may not fit into
a compiler diagnostic.
In addition, if the friend class ever uses the dtor, it'd trigger
-Wdelete-non-virtual-dtor.

Now to suppress the diagnostic, we have to add a `virtual`, wasting 2 entries
in the vtable and emitting unneeded D0/D2.

[Bug c++/102168] -Wnon-virtual-dtor shouldn't fire for protected dtor in a class with a friend declaration

[pdimov at gmail dot com]

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

Peter Dimov <pdimov at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |pdimov at gmail dot com

--- Comment #1 from Peter Dimov <pdimov at gmail dot com> ---
An issue against Boost.System has just been filed as a result of this warning:
https://github.com/boostorg/system/issues/83

It took me a while to figure out why the warning fires: because of the `friend
class error_code` declaration in `error_category`
(https://godbolt.org/z/z6x14P7M4).

Please reconsider issuing the warning when the destructor is protected and a
friend declaration exists. A protected destructor is a clear indication that
the type is intended as a base class.

[Bug c++/102168] -Wnon-virtual-dtor shouldn't fire for protected dtor in a class with a friend declaration

[redi at gcc dot gnu.org]

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

--- Comment #2 from Jonathan Wakely <redi at gcc dot gnu.org> ---
The -Wnon-virtual-dtor warning is just dumb, that's why I added Clang's
-Wdelete-non-virtual-dtor which should be used instead.

[Bug c++/102168] -Wnon-virtual-dtor shouldn't fire for protected dtor in a class with a friend declaration

[redi at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |81930
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=16168,
                   |                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=22395,
                   |                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=56879

--- Comment #3 from Jonathan Wakely <redi at gcc dot gnu.org> ---
The "there's a suitable friend" check was done intentionally, see r209056 which
was done to fix PR c++/56879


Referenced Bugs:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81930
[Bug 81930] [meta-bug] Issues with -Weffc++

[Bug c++/102168] -Wnon-virtual-dtor shouldn't fire for protected dtor in a class with a friend declaration

[pdimov at gmail dot com]

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

--- Comment #4 from Peter Dimov <pdimov at gmail dot com> ---
Warning on a private destructor + a friend declaration makes sense, because a
private destructor implies that the type is not intended to be derived from.
But warning on a protected destructor + a friend does not.

[Bug c++/102168] -Wnon-virtual-dtor shouldn't fire for protected dtor in a class with a friend declaration

[redi at gcc dot gnu.org]

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

--- Comment #5 from Jonathan Wakely <redi at gcc dot gnu.org> ---
But isn't the whole point of the warning to flag that "it is possible but
unsafe to delete an instance of a derived class through a pointer to the class
itself or base class." If it isn't intended to be derived from, that's not a
concern (there shouldn't be any derived classes). If it's intended to be
derived from, then there could be derived classes, and so an instance of such a
derived class could be deleted through a pointer to the base.

So isn't this exactly the case that the warning wants to warn about?

I'm not saying it's a good warning (I think it should never be used) but I
don't see how "the class is intended to be a base class" would make it
incorrect to warn about the possibility of deleting a derived class through a
pointer to base.

[Bug c++/102168] -Wnon-virtual-dtor shouldn't fire for protected dtor in a class with a friend declaration

[pdimov at gmail dot com]

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

--- Comment #6 from Peter Dimov <pdimov at gmail dot com> ---
Yes, I suppose you're right. The warning warns that Derived _can be_ deleted
via Base*, and that's correct - if not very useful in practice in this specific
case.

In fact the private destructor case is even less useful. ~Derived won't compile
unless it's Derived that is Base's friend, in which case everything is actually
fine.

I suppose we'll have to live with it. The annoying part is that there's no
warning, and then you add an unrelated friend declaration while refactoring,
and the warning appears.

[Bug c++/102168] -Wnon-virtual-dtor shouldn't fire for protected dtor in a class with a friend declaration

[redi at gcc dot gnu.org]

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

--- Comment #7 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Like I said, this warning should never be used :-)