From: benjamin priour <vultk...@gcc.gnu.org>
Hi,
This patch was the first I wrote and had been
at that time returned to me because ill-formatted.
Getting busy with other things, I forgot about it.
I've now fixed the formatting.
Succesfully regstrapped on x86_64-linux-gnu off trunk
a7d052b3200c7928d903a0242b8cfd75d131e374.
Is it OK for trunk ?
Thanks,
Benjamin.
Patch below.
---
Add a new warning for name-hiding. When a class's field
is named similarly to one inherited, a warning should
be issued.
This new warning is controlled by the existing Wshadow.
gcc/cp/ChangeLog:
PR c++/12341
* search.cc (lookup_member):
New optional parameter to preempt processing the
inheritance tree deeper than necessary.
(lookup_field): Likewise.
(dfs_walk_all): Likewise.
* cp-tree.h: Update the above declarations.
* class.cc: (warn_name_hiding): New function.
(finish_struct_1): Call warn_name_hiding if -Wshadow.
gcc/testsuite/ChangeLog:
PR c++/12341
* g++.dg/pr12341-1.C: New file.
* g++.dg/pr12341-2.C: New file.
Signed-off-by: benjamin priour <vultk...@gcc.gnu.org>
---
gcc/cp/class.cc | 75 ++++++++++++++++++++++++++++++++
gcc/cp/cp-tree.h | 9 ++--
gcc/cp/search.cc | 28 ++++++++----
gcc/testsuite/g++.dg/pr12341-1.C | 65 +++++++++++++++++++++++++++
gcc/testsuite/g++.dg/pr12341-2.C | 34 +++++++++++++++
5 files changed, 200 insertions(+), 11 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/pr12341-1.C
create mode 100644 gcc/testsuite/g++.dg/pr12341-2.C
diff --git a/gcc/cp/class.cc b/gcc/cp/class.cc
index 778759237dc..b1c59c392a0 100644
--- a/gcc/cp/class.cc
+++ b/gcc/cp/class.cc
@@ -3080,6 +3080,79 @@ warn_hidden (tree t)
}
}
+/* Warn about non-static fields name hiding. */
+
+static void
+warn_name_hiding (tree t)
+{
+ if (is_empty_class (t) || CLASSTYPE_NEARLY_EMPTY_P (t))
+ return;
+
+ for (tree field = TYPE_FIELDS (t); field; field = DECL_CHAIN (field))
+ {
+ /* Skip if field is not an user-defined non-static data member. */
+ if (TREE_CODE (field) != FIELD_DECL || DECL_ARTIFICIAL (field))
+ continue;
+
+ unsigned j;
+ tree name = DECL_NAME (field);
+ /* Skip if field is anonymous. */
+ if (!name || !identifier_p (name))
+ continue;
+
+ auto_vec<tree> base_vardecls;
+ tree binfo;
+ tree base_binfo;
+ /* Iterate through all of the base classes looking for possibly
+ shadowed non-static data members. */
+ for (binfo = TYPE_BINFO (t), j = 0;
+ BINFO_BASE_ITERATE (binfo, j, base_binfo); j++)
+ {
+ tree basetype = BINFO_TYPE (base_binfo);
+ tree candidate = lookup_field (basetype, name,
+ /* protect */ 2,
+ /* want_type */ 0,
+ /* once_suffices */ true);
+ if (candidate)
+ {
+ /* If we went up the hierarchy to a base class with multiple
+ inheritance, there could be multiple matches in which case
+ a TREE_LIST is returned. */
+ if (TREE_TYPE (candidate) == error_mark_node)
+ {
+ for (; candidate; candidate = TREE_CHAIN (candidate))
+ {
+ tree candidate_field = TREE_VALUE (candidate);
+ tree candidate_klass = DECL_CONTEXT (candidate_field);
+ if (accessible_base_p (t, candidate_klass, true))
+ base_vardecls.safe_push (candidate_field);
+ }
+ }
+ else if (accessible_base_p (t, DECL_CONTEXT (candidate), true))
+ base_vardecls.safe_push (candidate);
+ }
+ }
+
+ /* Field was not found among the base classes. */
+ if (base_vardecls.is_empty ())
+ continue;
+
+ /* Emit a warning for each field similarly named found
+ in the base class hierarchy. */
+ for (tree base_vardecl : base_vardecls)
+ {
+ if (base_vardecl)
+ {
+ auto_diagnostic_group d;
+ if (warning_at (location_of (field), OPT_Wshadow,
+ "%qD might shadow %qD", field, base_vardecl))
+ inform (location_of (base_vardecl),
+ " %qD name already in use here", base_vardecl);
+ }
+ }
+ }
+}
+
/* Recursive helper for finish_struct_anon. */
static void
@@ -7654,6 +7727,8 @@ finish_struct_1 (tree t)
if (warn_overloaded_virtual)
warn_hidden (t);
+ if (warn_shadow)
+ warn_name_hiding (t);
/* Class layout, assignment of virtual table slots, etc., is now
complete. Give the back end a chance to tweak the visibility of
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 3ca011c61c8..890326f0fd8 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -7554,11 +7554,13 @@ extern tree lookup_base (tree,
tree, base_access,
extern tree dcast_base_hint (tree, tree);
extern int accessible_p (tree, tree, bool);
extern int accessible_in_template_p (tree, tree);
-extern tree lookup_field (tree, tree, int, bool);
+extern tree lookup_field (tree, tree, int, bool,
+ bool once_suffices = false);
extern tree lookup_fnfields (tree, tree, int,
tsubst_flags_t);
extern tree lookup_member (tree, tree, int, bool,
tsubst_flags_t,
- access_failure_info *afi =
NULL);
+ access_failure_info *afi =
NULL,
+ bool once_suffices = false);
extern tree lookup_member_fuzzy (tree, tree, bool);
extern tree locate_field_accessor (tree, tree, bool);
extern int look_for_overrides (tree, tree);
@@ -7577,7 +7579,8 @@ extern tree binfo_for_vbase (tree,
tree);
extern tree look_for_overrides_here (tree, tree);
#define dfs_skip_bases ((tree)1)
extern tree dfs_walk_all (tree, tree (*) (tree, void *),
- tree (*) (tree, void *), void *);
+ tree (*) (tree, void *), void *,
+ bool stop_on_success = false);
extern tree dfs_walk_once (tree, tree (*) (tree, void *),
tree (*) (tree, void *), void *);
extern tree binfo_via_virtual (tree, tree);
diff --git a/gcc/cp/search.cc b/gcc/cp/search.cc
index cd80f285ac9..32bd35f3744 100644
--- a/gcc/cp/search.cc
+++ b/gcc/cp/search.cc
@@ -1145,13 +1145,19 @@ build_baselink (tree binfo, tree access_binfo, tree
functions, tree optype)
WANT_TYPE is 1 when we should only return TYPE_DECLs.
+ ONCE_SUFFICES is 1 when we should return upon first find of
+ the member in a branch of the inheritance hierarchy tree, rather
+ than collect all similarly named members further in that branch.
+ Does not impede other parallel branches of the tree.
+
If nothing can be found return NULL_TREE and do not issue an error.
If non-NULL, failure information is written back to AFI. */
tree
lookup_member (tree xbasetype, tree name, int protect, bool want_type,
- tsubst_flags_t complain, access_failure_info *afi /* = NULL */)
+ tsubst_flags_t complain, access_failure_info *afi /* = NULL */,
+ bool once_suffices /* = false */)
{
tree rval, rval_binfo = NULL_TREE;
tree type = NULL_TREE, basetype_path = NULL_TREE;
@@ -1202,7 +1208,7 @@ lookup_member (tree xbasetype, tree name, int protect,
bool want_type,
lfi.type = type;
lfi.name = name;
lfi.want_type = want_type;
- dfs_walk_all (basetype_path, &lookup_field_r, NULL, &lfi);
+ dfs_walk_all (basetype_path, &lookup_field_r, NULL, &lfi, once_suffices);
rval = lfi.rval;
rval_binfo = lfi.rval_binfo;
if (rval_binfo)
@@ -1392,10 +1398,12 @@ lookup_member_fuzzy (tree xbasetype, tree name, bool
want_type_p)
return NULL_TREE. */
tree
-lookup_field (tree xbasetype, tree name, int protect, bool want_type)
+lookup_field (tree xbasetype, tree name, int protect, bool want_type,
+ bool once_suffices /* = false */)
{
tree rval = lookup_member (xbasetype, name, protect, want_type,
- tf_warning_or_error);
+ tf_warning_or_error, /* afi */ NULL,
+ once_suffices);
/* Ignore functions, but propagate the ambiguity list. */
if (!error_operand_p (rval)
@@ -1480,11 +1488,14 @@ adjust_result_of_qualified_name_lookup (tree decl,
of PRE_FN and POST_FN can be NULL. At each node, PRE_FN and
POST_FN are passed the binfo to examine and the caller's DATA
value. All paths are walked, thus virtual and morally virtual
- binfos can be multiply walked. */
+ binfos can be multiply walked.
+ If STOP_ON_SUCCESS is 1, do not walk deeper the current hierarchy
+ tree once PRE_FN returns non-NULL. */
tree
dfs_walk_all (tree binfo, tree (*pre_fn) (tree, void *),
- tree (*post_fn) (tree, void *), void *data)
+ tree (*post_fn) (tree, void *), void *data,
+ bool stop_on_success /* = false */)
{
tree rval;
unsigned ix;
@@ -1496,7 +1507,7 @@ dfs_walk_all (tree binfo, tree (*pre_fn) (tree, void *),
rval = pre_fn (binfo, data);
if (rval)
{
- if (rval == dfs_skip_bases)
+ if (rval == dfs_skip_bases || stop_on_success)
goto skip_bases;
return rval;
}
@@ -1505,7 +1516,8 @@ dfs_walk_all (tree binfo, tree (*pre_fn) (tree, void *),
/* Find the next child binfo to walk. */
for (ix = 0; BINFO_BASE_ITERATE (binfo, ix, base_binfo); ix++)
{
- rval = dfs_walk_all (base_binfo, pre_fn, post_fn, data);
+ rval = dfs_walk_all (base_binfo, pre_fn, post_fn,
+ data, stop_on_success);
if (rval)
return rval;
}
diff --git a/gcc/testsuite/g++.dg/pr12341-1.C b/gcc/testsuite/g++.dg/pr12341-1.C
new file mode 100644
index 00000000000..d66b77a921d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pr12341-1.C
@@ -0,0 +1,65 @@
+// PR c++/12341
+/* { dg-do compile } */
+/* { dg-additional-options "-Wshadow" } */
+
+class A {
+protected:
+ int aaa; /* { dg-line A_def_aaa } */
+};
+
+// Check simple inheritance is checked for.
+class B : public A {
+public:
+ int aaa; /* { dg-line B_shadowing_aaa } */
+ /* { dg-warning "'B::aaa' might shadow 'A::aaa'" "" { target *-*-* } .-1 } */
+ /* { dg-note "'A::aaa' name already in use here" "" { target *-*-* }
A_def_aaa } */
+private:
+ int bbb; /* { dg-line B_def_bbb } */
+ int eee; /* { dg-line B_def_eee } */
+};
+
+// Check that visibility of base classes's fields is of no concern.
+class C : public B {
+public:
+ int bbb; /* { dg-warning "'C::bbb' might shadow 'B::bbb'" } */
+ /* { dg-note "'B::bbb' name already in use here" "" { target *-*-* }
B_def_bbb } */
+};
+
+class D {
+protected:
+ int bbb; /* { dg-line D_def_bbb } */
+ int ddd; /* { dg-line D_def_ddd } */
+};
+
+class E : protected D {
+private:
+ int eee; /* { dg-line E_def_eee } */
+};
+
+// all first-level base classes must be considered.
+class Bi : protected B, public E {
+protected:
+ /* Check that we stop on first ambiguous match,
+ that we don't walk the hierarchy deeper. */
+ int aaa; /* { dg_bogus "'Bi::aaa' might shadow 'A::aaa'" } */
+ /* { dg-warning "'Bi::aaa' might shadow 'B::aaa'" "" { target *-*-* } .-1 }
*/
+ /* { dg-note "'B::aaa' name already in use here" "" { target *-*-* }
B_shadowing_aaa } */
+
+ // All branches of a multiple inheritance tree must be explored.
+ int bbb; /* { dg-warning "'Bi::bbb' might shadow 'D::bbb'" } */
+ /* { dg-note "'D::bbb' name already in use here" "" { target *-*-* }
D_def_bbb } */
+ /* { dg-warning "'Bi::bbb' might shadow 'B::bbb'" "" { target *-*-* } .-2 }
*/
+
+ // Must continue beyond the immediate parent, even in the case of multiple
inheritance.
+ int ddd; /* { dg-warning "'Bi::ddd' might shadow 'D::ddd'" } */
+ /* { dg-note "'D::ddd' name already in use here" "" { target *-*-* }
D_def_ddd } */
+};
+
+class BiD : public Bi {
+ /* If the base class does not cause a warning,
+ then look into each base classes of the parent's multiple inheritance */
+ int eee; /* { dg-warning "'BiD::eee' might shadow 'B::eee'" } */
+ /* { dg-note "'B::eee' name already in use here" "" { target *-*-* }
B_def_eee } */
+ /* { dg-warning "'BiD::eee' might shadow 'E::eee'" "" { target *-*-* } .-2 }
*/
+ /* { dg-note "'E::eee' name already in use here" "" { target *-*-* }
E_def_eee } */
+};
diff --git a/gcc/testsuite/g++.dg/pr12341-2.C b/gcc/testsuite/g++.dg/pr12341-2.C
new file mode 100644
index 00000000000..2836ff780e5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/pr12341-2.C
@@ -0,0 +1,34 @@
+// PR c++/12341 test anonymous bit field
+/* { dg-do compile } */
+/* { dg-additional-options "-Wshadow" } */
+
+class B {
+protected:
+ unsigned int x : 5;
+ unsigned int : 2;
+ unsigned int y : 1;
+
+ union {
+ float uuu;
+ double vvv;
+ };
+};
+
+// Check that anonymous bit fields do not cause spurious warnings
+class D : private B {
+protected:
+ unsigned int x : 4; /* { dg-warning "'D::x' might shadow 'B::x'" } */
+ unsigned int : 4; // If test for excess errors fails, it might be from here.
+ int uuu; /* { dg-warning "'D::uuu' might shadow 'B::<unnamed union>::uuu'" }
*/
+};
+
+class E : public D {
+public:
+ /* Do not warn if inheritance is not visible to the class,
+ as there is no risk even with polymorphism to mistake the fields. */
+ unsigned int y; /* { dg-bogus "'E::y' might shadow 'B::y'" } */
+ unsigned int vvv; /* { dg-bogus "'E::vvv' might shadow 'B::<unnamed
union>::vvv'" } */
+
+ // Do warn even if the type differs. Should be trivial, still test for it.
+ double x; /* { dg-warning "'E::x' might shadow 'D::x'" } */
+};
--
2.34.1
