[clang] 3c42e10 - Consider aggregate bases when checking if an InitListExpr is constant (#80519)
via cfe-commits
cfe-commits at lists.llvm.org
Thu Feb 8 14:27:18 PST 2024
Author: Reid Kleckner
Date: 2024-02-08T14:27:14-08:00
New Revision: 3c42e10afdc518f6d8be5620289ef0da0bf03c5f
URL: https://github.com/llvm/llvm-project/commit/3c42e10afdc518f6d8be5620289ef0da0bf03c5f
DIFF: https://github.com/llvm/llvm-project/commit/3c42e10afdc518f6d8be5620289ef0da0bf03c5f.diff
LOG: 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 <aaron at aaronballman.com>
Added:
Modified:
clang/docs/ReleaseNotes.rst
clang/lib/AST/Expr.cpp
clang/test/SemaCXX/compound-literal.cpp
clang/unittests/AST/ASTExprTest.cpp
Removed:
################################################################################
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 32440ee64e3ebe..df3ad20f955eab 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/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
^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index d665a08deb47e6..8b10e289583260 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/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)
diff --git a/clang/test/SemaCXX/compound-literal.cpp b/clang/test/SemaCXX/compound-literal.cpp
index 5957099de53af3..a3d3b9faa9fee9 100644
--- a/clang/test/SemaCXX/compound-literal.cpp
+++ b/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 at +6 {{non-aggregate type 'Foo' cannot be initialized with an initializer list}}
+#elif __cplusplus < 201703L
+// expected-error at +4 {{no matching constructor}}
+#else
+// expected-error at +2 {{initializer element is not a compile-time constant}}
+#endif
+Foo o = (Foo){ {}, 1, f() };
+}
diff --git a/clang/unittests/AST/ASTExprTest.cpp b/clang/unittests/AST/ASTExprTest.cpp
index ec75492ccff8e4..5ec6aea8edba38 100644
--- a/clang/unittests/AST/ASTExprTest.cpp
+++ b/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,7 +62,7 @@ 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{}));
@@ -50,9 +70,41 @@ TEST(ASTExpr, IgnoreExprCallbackForwarded) {
{
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));
+}
More information about the cfe-commits
mailing list