Consider aggregate bases when checking if an InitListExpr is constant (#80519)

This code was correct as written prior to C++17, which allowed bases to
appear in the initializer list.

This was observable by creating non-constant aggregate initialization at
file scope in a compound literal, but since that behavior will change
soon if we implement support for dynamic initialization, I also added a
unit test for `isConstantInitializer`.

This fixes at least one part of issue #80510 .

---------

Co-authored-by: Aaron Ballman <[email protected]>

Author: rnk

clang/docs/ReleaseNotes.rst

@@ -217,6 +217,10 @@ Bug Fixes to C++ Support
   Fixes (`#80971 ICE when explicit object parameter be a function parameter pack`)
 - Fixed a bug where abbreviated function templates would append their invented template parameters to
   an empty template parameter lists.
+- Clang now classifies aggregate initialization in C++17 and newer as constant
+  or non-constant more accurately. Previously, only a subset of the initializer
+  elements were considered, misclassifying some initializers as constant. Fixes
+  some of (`#80510 <https://github.com/llvm/llvm-project/issues/80510>`).
 
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^

clang/lib/AST/Expr.cpp

@@ -3328,6 +3328,12 @@ bool Expr::isConstantInitializer(ASTContext &Ctx, bool IsForRef,
            DIUE->getUpdater()->isConstantInitializer(Ctx, false, Culprit);
   }
   case InitListExprClass: {
+    // C++ [dcl.init.aggr]p2:
+    //   The elements of an aggregate are:
+    //   - for an array, the array elements in increasing subscript order, or
+    //   - for a class, the direct base classes in declaration order, followed
+    //     by the direct non-static data members (11.4) that are not members of
+    //     an anonymous union, in declaration order.
     const InitListExpr *ILE = cast<InitListExpr>(this);
     assert(ILE->isSemanticForm() && "InitListExpr must be in semantic form");
     if (ILE->getType()->isArrayType()) {
@@ -3342,6 +3348,19 @@ bool Expr::isConstantInitializer(ASTContext &Ctx, bool IsForRef,
     if (ILE->getType()->isRecordType()) {
       unsigned ElementNo = 0;
       RecordDecl *RD = ILE->getType()->castAs<RecordType>()->getDecl();
+
+      // In C++17, bases were added to the list of members used by aggregate
+      // initialization.
+      if (const auto *CXXRD = dyn_cast<CXXRecordDecl>(RD)) {
+        for (unsigned i = 0, e = CXXRD->getNumBases(); i < e; i++) {
+          if (ElementNo < ILE->getNumInits()) {
+            const Expr *Elt = ILE->getInit(ElementNo++);
+            if (!Elt->isConstantInitializer(Ctx, false, Culprit))
+              return false;
+          }
+        }
+      }
+
       for (const auto *Field : RD->fields()) {
         // If this is a union, skip all the fields that aren't being initialized.
         if (RD->isUnion() && ILE->getInitializedFieldInUnion() != Field)

clang/test/SemaCXX/compound-literal.cpp

@@ -3,6 +3,7 @@
 // RUN: %clang_cc1 -fsyntax-only -std=c++11 -verify -ast-dump %s > %t-11
 // RUN: FileCheck --input-file=%t-11 %s
 // RUN: FileCheck --input-file=%t-11 %s --check-prefix=CHECK-CXX11
+// RUN: %clang_cc1 -verify -std=c++17 %s
 
 // http://llvm.org/PR7905
 namespace PR7905 {
@@ -108,3 +109,23 @@ int computed_with_lambda = [] {
   return result;
 }();
 #endif
+
+namespace DynamicFileScopeLiteral {
+// This covers the case where we have a file-scope compound literal with a
+// non-constant initializer in C++. Previously, we had a bug where Clang forgot
+// to consider initializer list elements for bases.
+struct Empty {};
+struct Foo : Empty { // expected-note 0+ {{candidate constructor}}
+  int x;
+  int y;
+};
+int f();
+#if __cplusplus < 201103L
+// expected-error@+6 {{non-aggregate type 'Foo' cannot be initialized with an initializer list}}
+#elif __cplusplus < 201703L
+// expected-error@+4 {{no matching constructor}}
+#else
+// expected-error@+2 {{initializer element is not a compile-time constant}}
+#endif
+Foo o = (Foo){ {}, 1, f() };
+}

clang/unittests/AST/ASTExprTest.cpp

@@ -20,17 +20,37 @@
 
 using namespace clang;
 
+using clang::ast_matchers::cxxRecordDecl;
+using clang::ast_matchers::hasName;
+using clang::ast_matchers::match;
+using clang::ast_matchers::varDecl;
+using clang::tooling::buildASTFromCode;
+
+static IntegerLiteral *createIntLiteral(ASTContext &Ctx, uint32_t Value) {
+  const int numBits = 32;
+  return IntegerLiteral::Create(Ctx, llvm::APInt(numBits, Value), Ctx.IntTy,
+                                {});
+}
+
+const CXXRecordDecl *getCXXRecordDeclNode(ASTUnit *AST,
+                                          const std::string &Name) {
+  auto Result =
+      match(cxxRecordDecl(hasName(Name)).bind("record"), AST->getASTContext());
+  EXPECT_FALSE(Result.empty());
+  return Result[0].getNodeAs<CXXRecordDecl>("record");
+}
+
+const VarDecl *getVariableNode(ASTUnit *AST, const std::string &Name) {
+  auto Result = match(varDecl(hasName(Name)).bind("var"), AST->getASTContext());
+  EXPECT_EQ(Result.size(), 1u);
+  return Result[0].getNodeAs<VarDecl>("var");
+}
+
 TEST(ASTExpr, IgnoreExprCallbackForwarded) {
   constexpr char Code[] = "";
   auto AST = tooling::buildASTFromCodeWithArgs(Code, /*Args=*/{"-std=c++20"});
   ASTContext &Ctx = AST->getASTContext();
 
-  auto createIntLiteral = [&](uint32_t Value) -> IntegerLiteral * {
-    const int numBits = 32;
-    return IntegerLiteral::Create(Ctx, llvm::APInt(numBits, Value),
-                                  Ctx.UnsignedIntTy, {});
-  };
-
   struct IgnoreParens {
     Expr *operator()(Expr *E) & { return nullptr; }
     Expr *operator()(Expr *E) && {
@@ -42,17 +62,49 @@ TEST(ASTExpr, IgnoreExprCallbackForwarded) {
   };
 
   {
-    auto *IntExpr = createIntLiteral(10);
+    auto *IntExpr = createIntLiteral(Ctx, 10);
     ParenExpr *PE =
         new (Ctx) ParenExpr(SourceLocation{}, SourceLocation{}, IntExpr);
     EXPECT_EQ(IntExpr, IgnoreExprNodes(PE, IgnoreParens{}));
   }
 
   {
     IgnoreParens CB{};
-    auto *IntExpr = createIntLiteral(10);
+    auto *IntExpr = createIntLiteral(Ctx, 10);
     ParenExpr *PE =
         new (Ctx) ParenExpr(SourceLocation{}, SourceLocation{}, IntExpr);
     EXPECT_EQ(nullptr, IgnoreExprNodes(PE, CB));
   }
 }
+
+TEST(ASTExpr, InitListIsConstantInitialized) {
+  auto AST = buildASTFromCode(R"cpp(
+    struct Empty {};
+    struct Foo : Empty { int x, y; };
+    int gv;
+  )cpp");
+  ASTContext &Ctx = AST->getASTContext();
+  const CXXRecordDecl *Empty = getCXXRecordDeclNode(AST.get(), "Empty");
+  const CXXRecordDecl *Foo = getCXXRecordDeclNode(AST.get(), "Foo");
+
+  SourceLocation Loc{};
+  InitListExpr *BaseInit = new (Ctx) InitListExpr(Ctx, Loc, {}, Loc);
+  BaseInit->setType(Ctx.getRecordType(Empty));
+  Expr *Exprs[3] = {
+      BaseInit,
+      createIntLiteral(Ctx, 13),
+      createIntLiteral(Ctx, 42),
+  };
+  InitListExpr *FooInit = new (Ctx) InitListExpr(Ctx, Loc, Exprs, Loc);
+  FooInit->setType(Ctx.getRecordType(Foo));
+  EXPECT_TRUE(FooInit->isConstantInitializer(Ctx, false));
+
+  // Replace the last initializer with something non-constant and make sure
+  // this returns false. Previously we had a bug where we didn't count base
+  // initializers, and only iterated over fields.
+  const VarDecl *GV = getVariableNode(AST.get(), "gv");
+  auto *Ref = new (Ctx) DeclRefExpr(Ctx, const_cast<VarDecl *>(GV), false,
+                                    Ctx.IntTy, VK_LValue, Loc);
+  (void)FooInit->updateInit(Ctx, 2, Ref);
+  EXPECT_FALSE(FooInit->isConstantInitializer(Ctx, false));
+}