On this example: ``` struct Fine { private: const int f; }; struct BuggyA { const int a; int &b; };
struct BuggyB : private BuggyA { }; ``` g++ currently emits: ``` test.cc:3:19: warning: non-static const member ‘const int Fine::f’ in class without a constructor [-Wuninitialized] 3 | const int f; | ``` (relevant godbolt: https://godbolt.org/z/KGMK6e1zc) The issue here is that g++ misses the uninitialized const and ref members in BuggyA that are inherited as private in BuggyB. It should warn about those members when checking BuggyB. With this patch, g++ emits the following: ``` test.cc:3:19: warning: non-static const member ‘const int Fine::f’ in class without a constructor [-Wuninitialized] 3 | const int f; | ^ test.cc:7:19: warning: while processing ‘BuggyB’: non-static const member ‘const int BuggyA::a’ in class without a constructor [-Wuninitialized] 7 | const int a; | ^ test.cc:7:19: note: ‘BuggyB’ inherits ‘BuggyA’ as private, so all fields contained within ‘BuggyA’ are private to ‘BuggyB’ test.cc:8:14: warning: while processing ‘BuggyB’: non-static reference ‘int& BuggyA::b’ in class without a constructor [-Wuninitialized] 8 | int &b; | ^ test.cc:8:14: note: ‘BuggyB’ inherits ‘BuggyA’ as private, so all fields contained within ‘BuggyA’ are private to ‘BuggyB’ ``` Now, the compiler warns about the uninitialized members. In terms of testing, I added three tests: - a status quo test that makes sure that the existing warning behavior works - A simple test based off of the PR - Another example with multiple inheritance - A final example with mutliple levels of inheritance. These tests all pass. I also bootstrapped the project without any regressions. PR c++/80681 gcc/cp/ChangeLog: * class.cc (warn_uninitialized_const_and_ref): Extract warn logic into new func, add inform for inheritance warning (check_bases_and_members): Move warn logic to warn_unintialized_const_and_ref, check subclasses for warnings as well gcc/testsuite/ChangeLog: * g++.dg/pr80681-1.C: New test. Signed-off-by: Charlie Sale <softwaresal...@gmail.com> --- gcc/cp/class.cc | 110 +++++++++++++++++++++++++------ gcc/testsuite/g++.dg/pr80681-1.C | 51 ++++++++++++++ 2 files changed, 142 insertions(+), 19 deletions(-) create mode 100644 gcc/testsuite/g++.dg/pr80681-1.C diff --git a/gcc/cp/class.cc b/gcc/cp/class.cc index aebcb53739e..72172bea6ad 100644 --- a/gcc/cp/class.cc +++ b/gcc/cp/class.cc @@ -6018,6 +6018,76 @@ explain_non_literal_class (tree t) } } + +/* Warn for private const or reference class members that cannot be initialized + due to the class not having a default constructor. If a child type is + provided, then we are checking class_type's members in case they cannot be + initialized by child_type. If child_type is null, then we simply check + class_type. */ +static void +warn_uninitialized_const_and_ref (tree class_type, tree child_type) +{ + /* Check the fields on this class type. */ + tree field; + for (field = TYPE_FIELDS (class_type); field; field = DECL_CHAIN (field)) + { + /* We only want to check variable declarations. + Exclude fields that are not field decls or are not initialized. */ + if (TREE_CODE (field) != FIELD_DECL + || DECL_INITIAL (field) != NULL_TREE) + continue; + + tree type = TREE_TYPE (field); + + if (TYPE_REF_P (type)) + { + if (child_type != nullptr) + { + /* Show parent class while processing. */ + auto_diagnostic_group d; + warning_at (DECL_SOURCE_LOCATION (field), + OPT_Wuninitialized, "while processing %qE: " + "non-static reference %q#D in class without a constructor", + child_type, field); + inform (DECL_SOURCE_LOCATION (field), + "%qE inherits %qE as private, so all fields " + "contained within %qE are private to %qE", + child_type, class_type, class_type, child_type); + } + else + { + warning_at (DECL_SOURCE_LOCATION (field), + OPT_Wuninitialized, "non-static reference %q#D " + "in class without a constructor", field); + } + } + else if (CP_TYPE_CONST_P (type) + && (!CLASS_TYPE_P (type) + || !TYPE_HAS_DEFAULT_CONSTRUCTOR (type))) + { + if (child_type) + { + /* ditto. */ + auto_diagnostic_group d; + warning_at (DECL_SOURCE_LOCATION (field), + OPT_Wuninitialized, "while processing %qE: " + "non-static const member %q#D in class " + "without a constructor", child_type, field); + inform (DECL_SOURCE_LOCATION (field), + "%qE inherits %qE as private, so all fields " + "contained within %qE are private to %qE", + child_type, class_type, class_type, child_type); + } + else + { + warning_at (DECL_SOURCE_LOCATION (field), + OPT_Wuninitialized, "non-static const member %q#D " + "in class without a constructor", field); + } + } + } +} + /* Check the validity of the bases and members declared in T. Add any implicitly-generated functions (like copy-constructors and assignment operators). Compute various flag bits (like @@ -6160,30 +6230,32 @@ check_bases_and_members (tree t) initializers. */ && CLASSTYPE_NON_AGGREGATE (t)) { - tree field; + /* First, warn for this class. */ + warn_uninitialized_const_and_ref (t, nullptr /* no child class. */); - for (field = TYPE_FIELDS (t); field; field = DECL_CHAIN (field)) - { - tree type; + /* Then, warn for any direct base classes. This process will not + descend all base classes, just the ones directly inherited by + this class. */ + tree binfo, base_binfo; + int idx; - if (TREE_CODE (field) != FIELD_DECL - || DECL_INITIAL (field) != NULL_TREE) - continue; + for (binfo = TYPE_BINFO (t), idx = 0; + BINFO_BASE_ITERATE (binfo, idx, base_binfo); idx++) + { + tree basetype = TREE_TYPE (base_binfo); - type = TREE_TYPE (field); - if (TYPE_REF_P (type)) - warning_at (DECL_SOURCE_LOCATION (field), - OPT_Wuninitialized, "non-static reference %q#D " - "in class without a constructor", field); - else if (CP_TYPE_CONST_P (type) - && (!CLASS_TYPE_P (type) - || !TYPE_HAS_DEFAULT_CONSTRUCTOR (type))) - warning_at (DECL_SOURCE_LOCATION (field), - OPT_Wuninitialized, "non-static const member %q#D " - "in class without a constructor", field); + /* Base types are not going to be aggregates, so don't + check for that. Otherwise, this step will never happen. */ + if (!TYPE_HAS_USER_CONSTRUCTOR (basetype) + || CLASSTYPE_NON_AGGREGATE (basetype)) + { + /* Repeat the same process on base classes, know that 't' + is a child_class of basetype. */ + warn_uninitialized_const_and_ref (basetype, t); + } } - } + } /* Synthesize any needed methods. */ add_implicitly_declared_members (t, &access_decls, cant_have_const_ctor, diff --git a/gcc/testsuite/g++.dg/pr80681-1.C b/gcc/testsuite/g++.dg/pr80681-1.C new file mode 100644 index 00000000000..40eab653aca --- /dev/null +++ b/gcc/testsuite/g++.dg/pr80681-1.C @@ -0,0 +1,51 @@ +// PR c++/80681 +// { dg-do compile } +// { dg-options "-Wuninitialized -c" } + +/* Status quo: the original should still warn correctly. */ +struct StatusQuo +{ +private: + const int a; // { dg-warning "non-static const member" } + int &b; // { dg-warning "non-static reference" } +}; + +/* Single layer of inheritance. */ +struct A { + const int a; // { dg-warning "non-static const member" } + int &b; // { dg-warning "non-static reference" } +}; + +struct B: private A { +}; + +/* multiple inheritance example. */ +struct X +{ + const int x; // { dg-warning "non-static const member" } +}; + +struct Y +{ + const int &y; // { dg-warning "non-static reference" } +}; + +struct Z : private Y, private X +{ +}; + +/* multi-level inheritance example. */ +struct M +{ + const int x; // { dg-warning "non-static const member" } +}; + +struct N: private M +{ + const int &y; // { dg-warning "non-static reference" } +}; + +struct O: private N +{ +}; + -- 2.38.1