public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jason Merrill <jason@redhat.com>
To: gcc-patches@gcc.gnu.org
Subject: [pushed] c++: Include -Woverloaded-virtual in -Wall [PR87729]
Date: Fri, 24 Jun 2022 18:26:04 -0400	[thread overview]
Message-ID: <20220624222604.335563-1-jason@redhat.com> (raw)

This seems like a good warning to have in -Wall, as requested.  But as
pointed out in PR20423, some users want a warning only when a derived
function doesn't override any base function.  So let's put that lesser
version in -Wall (and -Woverloaded-virtual=1) while leaving the semantics
for the existing option the same.

Tested x86_64-pc-linux-gnu, applying to trunk.

	PR c++/87729
	PR c++/20423

gcc/c-family/ChangeLog:

	* c.opt (Woverloaded-virtual): Add levels, include in -Wall.

gcc/ChangeLog:

	* doc/invoke.texi: Document changes.

gcc/cp/ChangeLog:

	* class.cc (warn_hidden): Handle -Woverloaded-virtual=1.

gcc/testsuite/ChangeLog:

	* g++.dg/warn/Woverloaded-virt1.C: New test.
	* g++.dg/warn/Woverloaded-virt2.C: New test.
---
 gcc/doc/invoke.texi                           | 26 ++++++++++++++++++-
 gcc/c-family/c.opt                            |  6 ++++-
 gcc/cp/class.cc                               | 17 +++++++++---
 gcc/testsuite/g++.dg/warn/Woverloaded-virt1.C | 14 ++++++++++
 gcc/testsuite/g++.dg/warn/Woverloaded-virt2.C | 15 +++++++++++
 5 files changed, 72 insertions(+), 6 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/warn/Woverloaded-virt1.C
 create mode 100644 gcc/testsuite/g++.dg/warn/Woverloaded-virt2.C

diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index f794edd49e1..dfaa561c8b8 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -4039,6 +4039,7 @@ a C++ program.  The new-style casts (@code{dynamic_cast},
 less vulnerable to unintended effects and much easier to search for.
 
 @item -Woverloaded-virtual @r{(C++ and Objective-C++ only)}
+@itemx -Woverloaded-virtual=@var{n}
 @opindex Woverloaded-virtual
 @opindex Wno-overloaded-virtual
 @cindex overloaded virtual function, warning
@@ -4052,7 +4053,7 @@ struct A @{
 @};
 
 struct B: public A @{
-  void f(int);
+  void f(int); // does not override
 @};
 @end smallexample
 
@@ -4067,6 +4068,29 @@ b->f();
 @noindent
 fails to compile.
 
+The optional level suffix controls the behavior when all the
+declarations in the derived class override virtual functions in the
+base class, even if not all of the base functions are overridden:
+
+@smallexample
+struct C @{
+  virtual void f();
+  virtual void f(int);
+@};
+
+struct D: public C @{
+  void f(int); // does override
+@}
+@end smallexample
+
+This pattern is less likely to be a mistake; if D is only used
+virtually, the user might have decided that the base class semantics
+for some of the overloads are fine.
+
+At level 1, this case does not warn; at level 2, it does.
+@option{-Woverloaded-virtual} by itself selects level 2.  Level 1 is
+included in @option{-Wall}.
+
 @item -Wno-pmf-conversions @r{(C++ and Objective-C++ only)}
 @opindex Wno-pmf-conversions
 @opindex Wpmf-conversions
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 41a20bc625e..44e1a60ce24 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -1126,7 +1126,11 @@ C ObjC C++ ObjC++ Var(warn_overlength_strings) Warning LangEnabledBy(C ObjC C++
 Warn if a string is longer than the maximum portable length specified by the standard.
 
 Woverloaded-virtual
-C++ ObjC++ Var(warn_overloaded_virtual) Warning
+C++ ObjC++ Warning Alias(Woverloaded-virtual=,2,0)
+Warn about overloaded virtual function names.
+
+Woverloaded-virtual=
+C++ ObjC++ Joined UInteger IntegerRange(0,2) Var(warn_overloaded_virtual) Warning LangEnabledBy(C++ ObjC++,Wall,1,0)
 Warn about overloaded virtual function names.
 
 Woverride-init
diff --git a/gcc/cp/class.cc b/gcc/cp/class.cc
index 3c195b35396..17683f421a7 100644
--- a/gcc/cp/class.cc
+++ b/gcc/cp/class.cc
@@ -3034,6 +3034,7 @@ warn_hidden (tree t)
 	  continue;
 
 	/* Remove any overridden functions.  */
+	bool seen_non_override = false;
 	for (tree fndecl : ovl_range (fns))
 	  {
 	    if (TREE_CODE (fndecl) == FUNCTION_DECL
@@ -3045,20 +3046,28 @@ warn_hidden (tree t)
 		for (size_t k = 0; k < base_fndecls.length (); k++)
 		  if (base_fndecls[k]
 		      && same_signature_p (fndecl, base_fndecls[k]))
-		    base_fndecls[k] = NULL_TREE;
+		    {
+		      base_fndecls[k] = NULL_TREE;
+		      goto next;
+		    }
 	      }
+	    seen_non_override = true;
+	  next:;
 	  }
 
+	if (!seen_non_override && warn_overloaded_virtual == 1)
+	  /* All the derived fns override base virtuals.  */
+	  return;
+
 	/* Now give a warning for all base functions without overriders,
 	   as they are hidden.  */
-	tree base_fndecl;
-	FOR_EACH_VEC_ELT (base_fndecls, j, base_fndecl)
+	for (tree base_fndecl : base_fndecls)
 	  if (base_fndecl)
 	    {
 	      auto_diagnostic_group d;
 	      /* Here we know it is a hider, and no overrider exists.  */
 	      if (warning_at (location_of (base_fndecl),
-			      OPT_Woverloaded_virtual,
+			      OPT_Woverloaded_virtual_,
 			      "%qD was hidden", base_fndecl))
 		inform (location_of (fns), "  by %qD", fns);
 	    }
diff --git a/gcc/testsuite/g++.dg/warn/Woverloaded-virt1.C b/gcc/testsuite/g++.dg/warn/Woverloaded-virt1.C
new file mode 100644
index 00000000000..92f8327b9d0
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Woverloaded-virt1.C
@@ -0,0 +1,14 @@
+// PR c++/87729
+// { dg-additional-options -Wall }
+
+class Foo
+{
+public:
+    virtual void f(int);	// { dg-warning "hidden" }
+};
+
+class Bar : public Foo
+{
+public:
+    virtual void f(short);	// { dg-message "by" }
+};
diff --git a/gcc/testsuite/g++.dg/warn/Woverloaded-virt2.C b/gcc/testsuite/g++.dg/warn/Woverloaded-virt2.C
new file mode 100644
index 00000000000..763ab29254e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Woverloaded-virt2.C
@@ -0,0 +1,15 @@
+// PR c++/20423
+// { dg-additional-options -Wall }
+
+class Foo
+{
+public:
+  virtual void f(int);
+  virtual void f(short);
+};
+
+class Bar : public Foo
+{
+public:
+  virtual void f(short);
+};

base-commit: 75fa80bb5654d1f8b21310118f41705b74168039
-- 
2.27.0


             reply	other threads:[~2022-06-24 22:26 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-24 22:26 Jason Merrill [this message]
2022-07-01 20:30 ` Stephan Bergmann
2022-07-05  8:18   ` Stephan Bergmann
2022-07-07 15:55 ` [pushed] c++: -Woverloaded-virtual and dtors [PR87729] Jason Merrill

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220624222604.335563-1-jason@redhat.com \
    --to=jason@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).