Hi, below is my first patch ever. I ran the testsuites against trunk 20230322, everything seems OK to me, but as it is my first submission I'd like to be sure of it. Thanks a lot for the review !
diff --git a/gcc/cp/class.cc b/gcc/cp/class.cc index 68b62086340..147a7458488 100644 --- a/gcc/cp/class.cc +++ b/gcc/cp/class.cc @@ -3080,6 +3080,109 @@ warn_hidden (tree t) } } + +/* Lookup the inheritance hierarchy of T for any non-static field that have + the indicated NAME. */ +static void +lookup_basevardecls (tree name, tree t, vec<tree> *base_vardecls) +{ + /* Find non-static fields in T with the indicated NAME. */ + for (tree field = TYPE_FIELDS(t); field; field = DECL_CHAIN(field)) + { + /* Going with the same policy as warn_hidden with base class's members + non visible from child, i.e. do not distinguish. + A justification might be to not omit warning in the following case: + + class Ancestor { + friend void foo_accessor(Ancestor *); + + private: + int x; + }; + + class Descendant : public Ancestor { + public: + int x; + } + + void foo_accessor(Ancestor *anc) { + ... + anc->x; + ... + } + + foo_accessor(new Descendant()); + */ + if (TREE_CODE (field) == FIELD_DECL + && !DECL_ARTIFICIAL(field) + && DECL_NAME(field) == name) + { + base_vardecls->safe_push (field); + /* Return upon first match, as there cannot be two data members + named equally in the same RECORD_TYPE. + Moreover, avoid redundant warnings by not diving deeper into + T inheritance hierarchy. */ + return; + } + } + + int n_baseclasses = BINFO_N_BASE_BINFOS (TYPE_BINFO (t)); + /* Go one step up the inheritance hierarchy. */ + for (int i = 0; i < n_baseclasses; i++) + { + tree basetype = BINFO_TYPE (BINFO_BASE_BINFO (TYPE_BINFO (t), i)); + lookup_basevardecls (name, basetype, base_vardecls); + } +} + + +/* 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); + /* Not sure about the size parameter of auto_vec */ + 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); + lookup_basevardecls(name, basetype, &base_vardecls); + } + + /* 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 +7757,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/testsuite/g++.dg/warn/pr12341-1.C b/gcc/testsuite/g++.dg/warn/pr12341-1.C new file mode 100644 index 00000000000..2c8f63d3b4f --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/pr12341-1.C @@ -0,0 +1,51 @@ +// PR c++/12341 +/* { dg-do compile } */ +/* { dg-options -Wshadow } */ + +class A { +protected: + int aaa; /* { dg-line A_def_aaa } */ +}; + +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 } */ +}; + +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; +}; + +// all first-level base classes must be considered. +class Bi : protected B, private E { +protected: + // If a match was already found, don't go further up the class hierarchy + 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 } */ + + // Every base classes 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 } */ +}; \ No newline at end of file