* [pushed] c++: Include -Woverloaded-virtual in -Wall [PR87729]
@ 2022-06-24 22:26 Jason Merrill
2022-07-01 20:30 ` Stephan Bergmann
2022-07-07 15:55 ` [pushed] c++: -Woverloaded-virtual and dtors [PR87729] Jason Merrill
0 siblings, 2 replies; 4+ messages in thread
From: Jason Merrill @ 2022-06-24 22:26 UTC (permalink / raw)
To: gcc-patches
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [pushed] c++: Include -Woverloaded-virtual in -Wall [PR87729]
2022-06-24 22:26 [pushed] c++: Include -Woverloaded-virtual in -Wall [PR87729] Jason Merrill
@ 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
1 sibling, 1 reply; 4+ messages in thread
From: Stephan Bergmann @ 2022-07-01 20:30 UTC (permalink / raw)
To: Jason Merrill, gcc-patches
On 6/25/22 00:26, Jason Merrill via Gcc-patches wrote:
> 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.
This now causes
> $ cat test.cc
> struct S1 {};
> struct S2: S1 { virtual ~S2(); };
> struct S3 { virtual ~S3(); };
> struct S4: S2, S3 { virtual ~S4(); };
> $ g++ -Woverloaded-virtual -fsyntax-only test.cc
> test.cc:3:21: warning: ‘virtual S3::~S3()’ was hidden [-Woverloaded-virtual=]
> 3 | struct S3 { virtual ~S3(); };
> | ^
> test.cc:4:29: note: by ‘virtual S4::~S4()’
> 4 | struct S4: S2, S3 { virtual ~S4(); };
> | ^
> test.cc:3:21: warning: ‘virtual S3::~S3()’ was hidden [-Woverloaded-virtual=]
> 3 | struct S3 { virtual ~S3(); };
> | ^
> test.cc:4:29: note: by ‘virtual S4::~S4()’
> 4 | struct S4: S2, S3 { virtual ~S4(); };
> | ^
> test.cc:3:21: warning: ‘virtual S3::~S3()’ was hidden [-Woverloaded-virtual=]
> 3 | struct S3 { virtual ~S3(); };
> | ^
> test.cc:4:29: note: by ‘virtual S4::~S4()’
> 4 | struct S4: S2, S3 { virtual ~S4(); };
> | ^
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [pushed] c++: Include -Woverloaded-virtual in -Wall [PR87729]
2022-07-01 20:30 ` Stephan Bergmann
@ 2022-07-05 8:18 ` Stephan Bergmann
0 siblings, 0 replies; 4+ messages in thread
From: Stephan Bergmann @ 2022-07-05 8:18 UTC (permalink / raw)
To: Jason Merrill, gcc-patches
On 01/07/2022 22:30, Stephan Bergmann wrote:
> On 6/25/22 00:26, Jason Merrill via Gcc-patches wrote:
>> 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.
>
> This now causes
[...]
moved that to a comment at
<https://gcc.gnu.org/bugzilla/show_bug.cgi?id=87729#c10> "Please include
-Woverloaded-virtual in -Wall" now
^ permalink raw reply [flat|nested] 4+ messages in thread
* [pushed] c++: -Woverloaded-virtual and dtors [PR87729]
2022-06-24 22:26 [pushed] c++: Include -Woverloaded-virtual in -Wall [PR87729] Jason Merrill
2022-07-01 20:30 ` Stephan Bergmann
@ 2022-07-07 15:55 ` Jason Merrill
1 sibling, 0 replies; 4+ messages in thread
From: Jason Merrill @ 2022-07-07 15:55 UTC (permalink / raw)
To: gcc-patches
My earlier patch broke out of the loop over base members when we found a
match, but that caused trouble for dtors, which can have multiple for which
same_signature_p is true. But as the function comment says, we know this
doesn't apply to [cd]tors, so skip them.
Tested x86_64-pc-linux-gnu, applying to trunk.
PR c++/87729
gcc/cp/ChangeLog:
* class.cc (warn_hidden): Ignore [cd]tors.
gcc/testsuite/ChangeLog:
* g++.dg/warn/Woverloaded-virt3.C: New test.
---
gcc/cp/class.cc | 3 +++
gcc/testsuite/g++.dg/warn/Woverloaded-virt3.C | 7 +++++++
2 files changed, 10 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/warn/Woverloaded-virt3.C
diff --git a/gcc/cp/class.cc b/gcc/cp/class.cc
index 17683f421a7..eb69e7f985c 100644
--- a/gcc/cp/class.cc
+++ b/gcc/cp/class.cc
@@ -3020,6 +3020,9 @@ warn_hidden (tree t)
tree binfo;
unsigned j;
+ if (IDENTIFIER_CDTOR_P (name))
+ continue;
+
/* Iterate through all of the base classes looking for possibly
hidden functions. */
for (binfo = TYPE_BINFO (t), j = 0;
diff --git a/gcc/testsuite/g++.dg/warn/Woverloaded-virt3.C b/gcc/testsuite/g++.dg/warn/Woverloaded-virt3.C
new file mode 100644
index 00000000000..34214ba2557
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Woverloaded-virt3.C
@@ -0,0 +1,7 @@
+// PR c++/87729
+// { dg-additional-options -Woverloaded-virtual }
+
+struct S1 {};
+struct S2: S1 { virtual ~S2(); };
+struct S3 { virtual ~S3(); };
+struct S4: S2, S3 { virtual ~S4(); };
base-commit: c1b1c4e58bda152ae932b45396ab67b07dd8c3fe
--
2.31.1
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-07-07 15:56 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-24 22:26 [pushed] c++: Include -Woverloaded-virtual in -Wall [PR87729] Jason Merrill
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
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).