* [PATCH 1/2] [C++] Remove tree_list from warn_hidden
@ 2014-11-27 8:53 tsaunders
2014-11-27 8:55 ` [PATCH 2/2] [C++] pr31397 - implement -Wsuggest-override tsaunders
2014-12-17 21:21 ` [PATCH 1/2] [C++] Remove tree_list from warn_hidden Jason Merrill
0 siblings, 2 replies; 7+ messages in thread
From: tsaunders @ 2014-11-27 8:53 UTC (permalink / raw)
To: gcc-patches; +Cc: jason, Trevor Saunders
From: Trevor Saunders <tsaunders@mozilla.com>
Hi,
I'll claim this is kind of a bug fix in so much as excessive gc allocation and
use of tree_list is a bug.
bootstrapped + regtested x86_64-unknown-linux-gnu, ok?
Trev
cp/
* class.c (warn_hidden): Use auto_vec<tree> instead of tree_list to
hold base_fndecls.
(get_basefndecls): Adjust.
diff --git a/gcc/cp/class.c b/gcc/cp/class.c
index c83c8ad..16279df 100644
--- a/gcc/cp/class.c
+++ b/gcc/cp/class.c
@@ -129,7 +129,7 @@ vec<tree, va_gc> *local_classes;
static tree get_vfield_name (tree);
static void finish_struct_anon (tree);
static tree get_vtable_name (tree);
-static tree get_basefndecls (tree, tree);
+static void get_basefndecls (tree, tree, vec<tree> *);
static int build_primary_vtable (tree, tree);
static int build_secondary_vtable (tree);
static void finish_vtbls (tree);
@@ -2717,16 +2717,16 @@ modify_all_vtables (tree t, tree virtuals)
/* Get the base virtual function declarations in T that have the
indicated NAME. */
-static tree
-get_basefndecls (tree name, tree t)
+static void
+get_basefndecls (tree name, tree t, vec<tree> *base_fndecls)
{
tree methods;
- tree base_fndecls = NULL_TREE;
int n_baseclasses = BINFO_N_BASE_BINFOS (TYPE_BINFO (t));
int i;
/* Find virtual functions in T with the indicated NAME. */
i = lookup_fnfields_1 (t, name);
+ bool found_decls = false;
if (i != -1)
for (methods = (*CLASSTYPE_METHOD_VEC (t))[i];
methods;
@@ -2736,20 +2736,20 @@ get_basefndecls (tree name, tree t)
if (TREE_CODE (method) == FUNCTION_DECL
&& DECL_VINDEX (method))
- base_fndecls = tree_cons (NULL_TREE, method, base_fndecls);
+ {
+ base_fndecls->safe_push (method);
+ found_decls = true;
+ }
}
- if (base_fndecls)
- return base_fndecls;
+ if (found_decls)
+ return;
for (i = 0; i < n_baseclasses; i++)
{
tree basetype = BINFO_TYPE (BINFO_BASE_BINFO (TYPE_BINFO (t), i));
- base_fndecls = chainon (get_basefndecls (name, basetype),
- base_fndecls);
+ get_basefndecls (name, basetype, base_fndecls);
}
-
- return base_fndecls;
}
/* If this declaration supersedes the declaration of
@@ -2811,7 +2811,6 @@ warn_hidden (tree t)
tree fn;
tree name;
tree fndecl;
- tree base_fndecls;
tree base_binfo;
tree binfo;
int j;
@@ -2820,19 +2819,18 @@ warn_hidden (tree t)
have the same name. Figure out what name that is. */
name = DECL_NAME (OVL_CURRENT (fns));
/* There are no possibly hidden functions yet. */
- base_fndecls = NULL_TREE;
+ auto_vec<tree, 20> base_fndecls;
/* Iterate through all of the base classes looking for possibly
hidden functions. */
for (binfo = TYPE_BINFO (t), j = 0;
BINFO_BASE_ITERATE (binfo, j, base_binfo); j++)
{
tree basetype = BINFO_TYPE (base_binfo);
- base_fndecls = chainon (get_basefndecls (name, basetype),
- base_fndecls);
+ get_basefndecls (name, basetype, &base_fndecls);
}
/* If there are no functions to hide, continue. */
- if (!base_fndecls)
+ if (!base_fndecls.exists ())
continue;
/* Remove any overridden functions. */
@@ -2842,28 +2840,27 @@ warn_hidden (tree t)
if (TREE_CODE (fndecl) == FUNCTION_DECL
&& DECL_VINDEX (fndecl))
{
- tree *prev = &base_fndecls;
-
- while (*prev)
/* If the method from the base class has the same
signature as the method from the derived class, it
has been overridden. */
- if (same_signature_p (fndecl, TREE_VALUE (*prev)))
- *prev = TREE_CHAIN (*prev);
- else
- prev = &TREE_CHAIN (*prev);
+ 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;
}
}
/* Now give a warning for all base functions without overriders,
as they are hidden. */
- while (base_fndecls)
- {
- /* Here we know it is a hider, and no overrider exists. */
- warning (OPT_Woverloaded_virtual, "%q+D was hidden", TREE_VALUE (base_fndecls));
- warning (OPT_Woverloaded_virtual, " by %q+D", fns);
- base_fndecls = TREE_CHAIN (base_fndecls);
- }
+ size_t k;
+ tree base_fndecl;
+ FOR_EACH_VEC_ELT (base_fndecls, k, base_fndecl)
+ if (base_fndecl)
+ {
+ /* Here we know it is a hider, and no overrider exists. */
+ warning (OPT_Woverloaded_virtual, "%q+D was hidden", base_fndecl);
+ warning (OPT_Woverloaded_virtual, " by %q+D", fns);
+ }
}
}
--
2.1.3
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] [C++] pr31397 - implement -Wsuggest-override
2014-11-27 8:53 [PATCH 1/2] [C++] Remove tree_list from warn_hidden tsaunders
@ 2014-11-27 8:55 ` tsaunders
2014-12-17 21:22 ` Jason Merrill
2014-12-17 21:21 ` [PATCH 1/2] [C++] Remove tree_list from warn_hidden Jason Merrill
1 sibling, 1 reply; 7+ messages in thread
From: tsaunders @ 2014-11-27 8:55 UTC (permalink / raw)
To: gcc-patches; +Cc: jason, Trevor Saunders
From: Trevor Saunders <tsaunders@mozilla.com>
Hi,
the idea in the pr of providing a warning that tells people where they can add
override seems reasonable to me.
bootstrapped + regtested x86_64-unknown--linux-gnu (new test passes), ok?
Trev
c-family/
* c.opt (Wsuggest-override): New option.
cp/
* class.c (check_for_override): Warn when a virtual function is an
override not marked override.
gcc/
* doc/invoke.texi: Document -Wsuggest-override.
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 85dcb98..259b520 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -574,6 +574,11 @@ Wsuggest-attribute=format
C ObjC C++ ObjC++ Var(warn_suggest_attribute_format) Warning
Warn about functions which might be candidates for format attributes
+Wsuggest-override
+C++ ObjC++ Var(warn_override) Warning
+Suggest that the override keyword be used when the declaration of a virtual
+function overrides another.
+
Wswitch
C ObjC C++ ObjC++ Var(warn_switch) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall)
Warn about enumerated switches, with no default, missing a case
diff --git a/gcc/cp/class.c b/gcc/cp/class.c
index 16279df..515f33f 100644
--- a/gcc/cp/class.c
+++ b/gcc/cp/class.c
@@ -2777,6 +2777,10 @@ check_for_override (tree decl, tree ctype)
{
DECL_VINDEX (decl) = decl;
overrides_found = true;
+ if (warn_override && DECL_VIRTUAL_P (decl) && !DECL_OVERRIDE_P (decl)
+ && !DECL_DESTRUCTOR_P (decl))
+ warning_at(DECL_SOURCE_LOCATION (decl), OPT_Wsuggest_override,
+ "%q+D can be marked override", decl);
}
if (DECL_VIRTUAL_P (decl))
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 89edddb..8741e8e 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -275,7 +275,7 @@ Objective-C and Objective-C++ Dialects}.
-Wstack-protector -Wstack-usage=@var{len} -Wstrict-aliasing @gol
-Wstrict-aliasing=n @gol -Wstrict-overflow -Wstrict-overflow=@var{n} @gol
-Wsuggest-attribute=@r{[}pure@r{|}const@r{|}noreturn@r{|}format@r{]} @gol
--Wsuggest-final-types @gol -Wsuggest-final-methods @gol
+-Wsuggest-final-types @gol -Wsuggest-final-methods @gol -Wsuggest-override @gol
-Wmissing-format-attribute @gol
-Wswitch -Wswitch-default -Wswitch-enum -Wswitch-bool -Wsync-nand @gol
-Wsystem-headers -Wtrampolines -Wtrigraphs -Wtype-limits -Wundef @gol
@@ -4255,6 +4255,10 @@ effective with link time optimization, where the information about the class
hiearchy graph is more complete. It is recommended to first consider suggestins
of @option{-Wsuggest-final-types} and then rebuild with new annotations.
+@item -Wsuggest-override
+Warn about overriding virtual functions that are not marked with the override
+keyword.
+
@item -Warray-bounds
@opindex Wno-array-bounds
@opindex Warray-bounds
diff --git a/gcc/testsuite/g++.dg/warn/Wsuggest-override.C b/gcc/testsuite/g++.dg/warn/Wsuggest-override.C
new file mode 100644
index 0000000..929d365
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wsuggest-override.C
@@ -0,0 +1,21 @@
+// { dg-do compile }
+// { dg-options "-std=c++11 -Wsuggest-override" }
+struct A
+{
+ A();
+ virtual ~A();
+ virtual void f();
+ virtual int bar();
+ operator int();
+ virtual operator float();
+};
+
+struct B : A
+{
+ B();
+ virtual ~B();
+ virtual void f(); // { dg-warning "can be marked override" }
+virtual int bar() override;
+operator int();
+virtual operator float(); // { dg-warning "can be marked override" }
+};
--
2.1.3
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] [C++] Remove tree_list from warn_hidden
2014-11-27 8:53 [PATCH 1/2] [C++] Remove tree_list from warn_hidden tsaunders
2014-11-27 8:55 ` [PATCH 2/2] [C++] pr31397 - implement -Wsuggest-override tsaunders
@ 2014-12-17 21:21 ` Jason Merrill
1 sibling, 0 replies; 7+ messages in thread
From: Jason Merrill @ 2014-12-17 21:21 UTC (permalink / raw)
To: tsaunders, gcc-patches
On 11/27/2014 01:28 AM, tsaunders@mozilla.com wrote:
> - if (!base_fndecls)
> + if (!base_fndecls.exists ())
This should use is_empty rather than exists. OK with that change.
Jason
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] [C++] pr31397 - implement -Wsuggest-override
2014-11-27 8:55 ` [PATCH 2/2] [C++] pr31397 - implement -Wsuggest-override tsaunders
@ 2014-12-17 21:22 ` Jason Merrill
2014-12-18 17:00 ` Trevor Saunders
2014-12-24 8:36 ` [PATCH] " tbsaunde+gcc
0 siblings, 2 replies; 7+ messages in thread
From: Jason Merrill @ 2014-12-17 21:22 UTC (permalink / raw)
To: tsaunders, gcc-patches
On 11/27/2014 01:28 AM, tsaunders@mozilla.com wrote:
> + if (warn_override && DECL_VIRTUAL_P (decl) && !DECL_OVERRIDE_P (decl)
Why check DECL_VIRTUAL_P here?
> + warning_at(DECL_SOURCE_LOCATION (decl), OPT_Wsuggest_override,
Space before the (.
Jason
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] [C++] pr31397 - implement -Wsuggest-override
2014-12-17 21:22 ` Jason Merrill
@ 2014-12-18 17:00 ` Trevor Saunders
2014-12-24 8:36 ` [PATCH] " tbsaunde+gcc
1 sibling, 0 replies; 7+ messages in thread
From: Trevor Saunders @ 2014-12-18 17:00 UTC (permalink / raw)
To: gcc-patches
On Wed, Dec 17, 2014 at 04:21:31PM -0500, Jason Merrill wrote:
> On 11/27/2014 01:28 AM, tsaunders@mozilla.com wrote:
> >+ if (warn_override && DECL_VIRTUAL_P (decl) && !DECL_OVERRIDE_P (decl)
>
> Why check DECL_VIRTUAL_P here?
I think it was to avoid warning when decl is hiding a non virtual method
on the parent class, but I guess that won't happen anyway? so I'll
remove it.
Trev
>
> >+ warning_at(DECL_SOURCE_LOCATION (decl), OPT_Wsuggest_override,
>
> Space before the (.
>
> Jason
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] pr31397 - implement -Wsuggest-override
2014-12-17 21:22 ` Jason Merrill
2014-12-18 17:00 ` Trevor Saunders
@ 2014-12-24 8:36 ` tbsaunde+gcc
2014-12-25 15:15 ` Jason Merrill
1 sibling, 1 reply; 7+ messages in thread
From: tbsaunde+gcc @ 2014-12-24 8:36 UTC (permalink / raw)
To: jason; +Cc: gcc-patches, Trevor Saunders
From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org>
Hi,
comments fixed. bootstrapped on x86_64-linux, new test passes and regtest
pending, ok?
Trev
c-family/
* c.opt (Wsuggest-override): New option.
cp/
* class.c (check_for_override): Warn when a virtual function is an
override not marked override.
gcc/
* doc/invoke.texi: Document -Wsuggest-override.
---
gcc/c-family/c.opt | 5 +++++
gcc/cp/class.c | 4 ++++
gcc/doc/invoke.texi | 6 +++++-
gcc/testsuite/g++.dg/warn/Wsuggest-override.C | 23 +++++++++++++++++++++++
4 files changed, 37 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.dg/warn/Wsuggest-override.C
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index f86718b..1f6f793 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -578,6 +578,11 @@ Wsuggest-attribute=format
C ObjC C++ ObjC++ Var(warn_suggest_attribute_format) Warning
Warn about functions which might be candidates for format attributes
+Wsuggest-override
+C++ ObjC++ Var(warn_override) Warning
+Suggest that the override keyword be used when the declaration of a virtual
+function overrides another.
+
Wswitch
C ObjC C++ ObjC++ Var(warn_switch) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall)
Warn about enumerated switches, with no default, missing a case
diff --git a/gcc/cp/class.c b/gcc/cp/class.c
index 0ac2124..d2cf4a0 100644
--- a/gcc/cp/class.c
+++ b/gcc/cp/class.c
@@ -2811,6 +2811,10 @@ check_for_override (tree decl, tree ctype)
{
DECL_VINDEX (decl) = decl;
overrides_found = true;
+ if (warn_override && !DECL_OVERRIDE_P (decl)
+ && !DECL_DESTRUCTOR_P (decl))
+ warning_at (DECL_SOURCE_LOCATION (decl), OPT_Wsuggest_override,
+ "%q+D can be marked override", decl);
}
if (DECL_VIRTUAL_P (decl))
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 9f56f42..d0ee093 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -277,7 +277,7 @@ Objective-C and Objective-C++ Dialects}.
-Wstack-protector -Wstack-usage=@var{len} -Wstrict-aliasing @gol
-Wstrict-aliasing=n @gol -Wstrict-overflow -Wstrict-overflow=@var{n} @gol
-Wsuggest-attribute=@r{[}pure@r{|}const@r{|}noreturn@r{|}format@r{]} @gol
--Wsuggest-final-types @gol -Wsuggest-final-methods @gol
+-Wsuggest-final-types @gol -Wsuggest-final-methods @gol -Wsuggest-override @gol
-Wmissing-format-attribute @gol
-Wswitch -Wswitch-default -Wswitch-enum -Wswitch-bool -Wsync-nand @gol
-Wsystem-headers -Wtrampolines -Wtrigraphs -Wtype-limits -Wundef @gol
@@ -4276,6 +4276,10 @@ class hierarchy graph is more complete. It is recommended to first consider
suggestions of @option{-Wsuggest-final-types} and then rebuild with new
annotations.
+@item -Wsuggest-override
+Warn about overriding virtual functions that are not marked with the override
+keyword.
+
@item -Warray-bounds
@opindex Wno-array-bounds
@opindex Warray-bounds
diff --git a/gcc/testsuite/g++.dg/warn/Wsuggest-override.C b/gcc/testsuite/g++.dg/warn/Wsuggest-override.C
new file mode 100644
index 0000000..f820f4b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wsuggest-override.C
@@ -0,0 +1,23 @@
+// { dg-do compile }
+// { dg-options "-std=c++11 -Wsuggest-override" }
+struct A
+{
+ A();
+ virtual ~A();
+ virtual void f();
+ virtual int bar();
+ int c();
+ operator int();
+ virtual operator float();
+};
+
+struct B : A
+{
+ B();
+ virtual ~B();
+ virtual void f(); // { dg-warning "can be marked override" }
+virtual int bar() override;
+int c();
+operator int();
+virtual operator float(); // { dg-warning "can be marked override" }
+};
--
2.1.3
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] pr31397 - implement -Wsuggest-override
2014-12-24 8:36 ` [PATCH] " tbsaunde+gcc
@ 2014-12-25 15:15 ` Jason Merrill
0 siblings, 0 replies; 7+ messages in thread
From: Jason Merrill @ 2014-12-25 15:15 UTC (permalink / raw)
To: tbsaunde+gcc; +Cc: gcc-patches
OK.
Jason
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-12-25 7:31 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-11-27 8:53 [PATCH 1/2] [C++] Remove tree_list from warn_hidden tsaunders
2014-11-27 8:55 ` [PATCH 2/2] [C++] pr31397 - implement -Wsuggest-override tsaunders
2014-12-17 21:22 ` Jason Merrill
2014-12-18 17:00 ` Trevor Saunders
2014-12-24 8:36 ` [PATCH] " tbsaunde+gcc
2014-12-25 15:15 ` Jason Merrill
2014-12-17 21:21 ` [PATCH 1/2] [C++] Remove tree_list from warn_hidden 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).