https://github.com/rnk updated https://github.com/llvm/llvm-project/pull/80519
>From 6ab5ba3f970eaaea542fbed09cae17d3666df6b3 Mon Sep 17 00:00:00 2001 From: Reid Kleckner <r...@google.com> Date: Sat, 3 Feb 2024 00:18:42 +0000 Subject: [PATCH 1/3] wip --- clang/lib/AST/Expr.cpp | 12 ++++++++++++ clang/test/SemaCXX/compound-literal.cpp | 20 ++++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp index d665a08deb47e6..8852fadf79b9ac 100644 --- a/clang/lib/AST/Expr.cpp +++ b/clang/lib/AST/Expr.cpp @@ -3342,6 +3342,18 @@ bool Expr::isConstantInitializer(ASTContext &Ctx, bool IsForRef, if (ILE->getType()->isRecordType()) { unsigned ElementNo = 0; RecordDecl *RD = ILE->getType()->castAs<RecordType>()->getDecl(); + + // Check bases for C++17 aggregate initializers. + 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..81f8b41ff0313e 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,22 @@ int computed_with_lambda = [] { return result; }(); #endif + +#if __cplusplus >= 201703L +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 { + int x; + int y; +}; +int f(); +Foo o = (Foo){ + {}, + 1, + f() // expected-error {{initializer element is not a compile-time constant}} +}; +} +#endif >From c630eee1f930c87d2c461de92271c02de220b290 Mon Sep 17 00:00:00 2001 From: Reid Kleckner <r...@google.com> Date: Mon, 5 Feb 2024 23:47:57 +0000 Subject: [PATCH 2/3] update comments, run test in past language modes, and add unit test --- clang/lib/AST/Expr.cpp | 9 +++- clang/test/SemaCXX/compound-literal.cpp | 17 ++++--- clang/unittests/AST/ASTExprTest.cpp | 66 ++++++++++++++++++++++--- 3 files changed, 75 insertions(+), 17 deletions(-) diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp index 8852fadf79b9ac..9190995c1df20e 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++ [temp.dep.expr]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()) { @@ -3343,7 +3349,8 @@ bool Expr::isConstantInitializer(ASTContext &Ctx, bool IsForRef, unsigned ElementNo = 0; RecordDecl *RD = ILE->getType()->castAs<RecordType>()->getDecl(); - // Check bases for C++17 aggregate initializers. + // 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()) { diff --git a/clang/test/SemaCXX/compound-literal.cpp b/clang/test/SemaCXX/compound-literal.cpp index 81f8b41ff0313e..a3d3b9faa9fee9 100644 --- a/clang/test/SemaCXX/compound-literal.cpp +++ b/clang/test/SemaCXX/compound-literal.cpp @@ -110,21 +110,22 @@ int computed_with_lambda = [] { }(); #endif -#if __cplusplus >= 201703L 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 { +struct Foo : Empty { // expected-note 0+ {{candidate constructor}} int x; int y; }; int f(); -Foo o = (Foo){ - {}, - 1, - f() // expected-error {{initializer element is not a compile-time constant}} -}; -} +#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() }; +} diff --git a/clang/unittests/AST/ASTExprTest.cpp b/clang/unittests/AST/ASTExprTest.cpp index ec75492ccff8e4..257ac9c55d1bb0 100644 --- a/clang/unittests/AST/ASTExprTest.cpp +++ b/clang/unittests/AST/ASTExprTest.cpp @@ -20,17 +20,35 @@ 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 +60,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 +68,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)); +} >From 07b4bf03a2b95be7e4de7c6b2fb0d09b89befc02 Mon Sep 17 00:00:00 2001 From: Reid Kleckner <r...@google.com> Date: Tue, 6 Feb 2024 00:25:59 +0000 Subject: [PATCH 3/3] Avoid emdash, use an ASCII hyphen --- clang/lib/AST/Expr.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp index 9190995c1df20e..dec9e16ed78290 100644 --- a/clang/lib/AST/Expr.cpp +++ b/clang/lib/AST/Expr.cpp @@ -3330,8 +3330,8 @@ bool Expr::isConstantInitializer(ASTContext &Ctx, bool IsForRef, case InitListExprClass: { // C++ [temp.dep.expr]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 + // - 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); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits