* [PATCH] c++: Additional warning for name-hiding [PR12341]
@ 2023-04-16 21:33 Benjamin Priour
0 siblings, 0 replies; 3+ messages in thread
From: Benjamin Priour @ 2023-04-16 21:33 UTC (permalink / raw)
To: gcc-patches, Jason Merrill
Hi everyone,
My first patch, and I don't have write access yet.
This patch add a new warning under -Wshadow, to warn when a class
field hides another inherited.
At the moment, I'm looking for a similarly
named field independently of its visibility (whether it is public,
protected or private within the base class).
However, if the inheritance itself is not visible from the current
class, then I dismiss the warning
(e.g. Grandparent class is inherited privately by Parent, then Child
won't collide).
Note that ChangeLogs were generated without the script `git gcc-mklog`.
Bootstrapped and regtested on x86_64-pc-linux-gnu.
Diff is with 20230413-trunk, I checked in with today 20230416 trunk though.
I tried to follow the contribute page down to the letter, still if
I've missed anything, please tell me how I could improve the
submission.
Great thanks,
Benjamin.
c++: Additional warning for name-hiding [PR12341]
Add a new warning for name-hiding. When a class's field
is named similarly to one inherited, a warning should
be issued.
2023-04-13 Benjamin Priour vutlkayn@gcc.gnu.org
gcc/cp/ChangeLog:
* search.cc (lookup_member):
New optional parameter to preempt too deep inheritance
tree processing.
(lookup_field): Likewise.
(dfs_walk_all): Likewise.
* cp-tree.h: Complete the above declarations.
* class.cc (warn_name_hiding): New function.
(finish_struct_1): Call warn_name_hiding if -Wshadow.
gcc/testsuite/ChangeLog:
* g++.dg/warn/pr12341-1.C: New file.
* g++.dg/warn/pr12341-2.C: New file.
---
diff --git a/gcc/cp/class.cc b/gcc/cp/class.cc
index 68b62086340..1e3efc028a6 100644
--- a/gcc/cp/class.cc
+++ b/gcc/cp/class.cc
@@ -3080,6 +3080,80 @@ 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 a 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 +7728,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 622752ae4e6..e56e85d93cc 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -7519,11 +7519,11 @@ 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);
@@ -7542,7 +7542,7 @@ 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 3f521b3bd72..7dbdb962aee 100644
--- a/gcc/cp/search.cc
+++ b/gcc/cp/search.cc
@@ -1126,13 +1126,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;
@@ -1183,7 +1189,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)
@@ -1373,10 +1379,11 @@ 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)
@@ -1461,11 +1468,13 @@ 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;
@@ -1477,7 +1486,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;
}
@@ -1486,7 +1495,7 @@ 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/warn/pr12341-1.C
b/gcc/testsuite/g++.dg/warn/pr12341-1.C
new file mode 100644
index 00000000000..eac0351686d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/pr12341-1.C
@@ -0,0 +1,65 @@
+// PR c++/12341
+/* { dg-do compile } */
+/* { dg-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 } */
+};
\ No newline at end of file
diff --git a/gcc/testsuite/g++.dg/warn/pr12341-2.C
b/gcc/testsuite/g++.dg/warn/pr12341-2.C
new file mode 100644
index 00000000000..6a24d094cea
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/pr12341-2.C
@@ -0,0 +1,34 @@
+// PR c++/12341 test anonymous bit field
+/* { dg-do compile } */
+/* { dg-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'" } */
+};
--
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] c++: Additional warning for name-hiding [PR12341]
2023-09-04 17:18 priour.be
@ 2023-09-05 20:52 ` Jason Merrill
0 siblings, 0 replies; 3+ messages in thread
From: Jason Merrill @ 2023-09-05 20:52 UTC (permalink / raw)
To: priour.be, gcc-patches; +Cc: benjamin priour
On 9/4/23 13:18, priour.be@gmail.com wrote:
> From: benjamin priour <vultkayn@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 <vultkayn@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++)
Rather than iterate through the bases here, maybe add a mode to
lookup_member/lookup_field_r that skips the most derived type, e.g. by
adding that as a flag in lookup_field_info?
Probably instead of the once_suffices stuff?
> + if (base_vardecl)
> + {
> + auto_diagnostic_group d;
> + if (warning_at (location_of (field), OPT_Wshadow,
> + "%qD might shadow %qD", field, base_vardecl))
Why "might"? We can give a correct answer, we shouldn't settle for an
approximation.
Jason
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH] c++: Additional warning for name-hiding [PR12341]
@ 2023-09-04 17:18 priour.be
2023-09-05 20:52 ` Jason Merrill
0 siblings, 1 reply; 3+ messages in thread
From: priour.be @ 2023-09-04 17:18 UTC (permalink / raw)
To: gcc-patches; +Cc: jason, benjamin priour
From: benjamin priour <vultkayn@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 <vultkayn@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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-09-05 20:52 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-16 21:33 [PATCH] c++: Additional warning for name-hiding [PR12341] Benjamin Priour
2023-09-04 17:18 priour.be
2023-09-05 20:52 ` 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).