cp: warn uninitialized const/ref in base class [PR80681]

Message ID 20221224190343.3490-1-softwaresale01@gmail.com
State Accepted
Headers
Series cp: warn uninitialized const/ref in base class [PR80681] |

Checks

Context Check Description
snail/gcc-patch-check success Github commit url

Commit Message

Charlie Sale Dec. 24, 2022, 7:03 p.m. UTC
  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
  

Patch

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
+{
+};
+