[ping] cp: warn uninitialized const/ref in base class [PR80681]
Commit Message
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 <softwaresale01@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
@@ -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,
new file mode 100644
@@ -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
+{
+};
+