[clang] [Clang] Correctly initialize placeholder fields from their initializers (PR #114196)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Nov 5 09:10:42 PST 2024
https://github.com/cor3ntin updated https://github.com/llvm/llvm-project/pull/114196
>From b224993200d2ac7a075c7b112b7d98c38078c9fc Mon Sep 17 00:00:00 2001
From: Corentin Jabot <corentinjabot at gmail.com>
Date: Wed, 30 Oct 2024 10:25:42 +0100
Subject: [PATCH] [Clang] Correctly initialize placeholder fields from their
initializers
We made the incorrect assumption that names of fields are unique
when creating their default initializers.
We fix that by keeping track of the instantiaation pattern
for field decls that are placeholder vars,
like we already do for unamed fields.
Fixes #114069
---
clang/docs/ReleaseNotes.rst | 1 +
clang/include/clang/AST/ASTContext.h | 2 +-
clang/lib/AST/ASTContext.cpp | 9 +++--
clang/lib/Sema/SemaExpr.cpp | 29 ++++++++++-----
.../lib/Sema/SemaTemplateInstantiateDecl.cpp | 2 +-
clang/lib/Serialization/ASTReaderDecl.cpp | 3 +-
clang/lib/Serialization/ASTWriterDecl.cpp | 2 +-
clang/test/SemaCXX/cxx2c-placeholder-vars.cpp | 36 ++++++++++++++++++-
8 files changed, 67 insertions(+), 17 deletions(-)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 6085352dfafe6b..eea757b8334150 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -574,6 +574,7 @@ Bug Fixes to C++ Support
(#GH95854).
- Fixed an assertion failure when evaluating an invalid expression in an array initializer. (#GH112140)
- Fixed an assertion failure in range calculations for conditional throw expressions. (#GH111854)
+- Name independent data members were not correctly initialized from default member initializers. (#GH114069)
Bug Fixes to AST Handling
^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h
index 07b4e36f3ef05e..aa3c3734cef166 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -1037,7 +1037,7 @@ class ASTContext : public RefCountedBase<ASTContext> {
void setInstantiatedFromUsingShadowDecl(UsingShadowDecl *Inst,
UsingShadowDecl *Pattern);
- FieldDecl *getInstantiatedFromUnnamedFieldDecl(FieldDecl *Field);
+ FieldDecl *getInstantiatedFromUnnamedFieldDecl(FieldDecl *Field) const;
void setInstantiatedFromUnnamedFieldDecl(FieldDecl *Inst, FieldDecl *Tmpl);
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 1c3f771f417ccf..7dd646c3a50b34 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -1583,14 +1583,17 @@ ASTContext::setInstantiatedFromUsingShadowDecl(UsingShadowDecl *Inst,
InstantiatedFromUsingShadowDecl[Inst] = Pattern;
}
-FieldDecl *ASTContext::getInstantiatedFromUnnamedFieldDecl(FieldDecl *Field) {
+FieldDecl *
+ASTContext::getInstantiatedFromUnnamedFieldDecl(FieldDecl *Field) const {
return InstantiatedFromUnnamedFieldDecl.lookup(Field);
}
void ASTContext::setInstantiatedFromUnnamedFieldDecl(FieldDecl *Inst,
FieldDecl *Tmpl) {
- assert(!Inst->getDeclName() && "Instantiated field decl is not unnamed");
- assert(!Tmpl->getDeclName() && "Template field decl is not unnamed");
+ assert((!Inst->getDeclName() || Inst->isPlaceholderVar(getLangOpts())) &&
+ "Instantiated field decl is not unnamed");
+ assert((!Inst->getDeclName() || Inst->isPlaceholderVar(getLangOpts())) &&
+ "Template field decl is not unnamed");
assert(!InstantiatedFromUnnamedFieldDecl[Inst] &&
"Already noted what unnamed field was instantiated from");
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index ff6616901016ab..bfd1414c44fc28 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -5560,6 +5560,24 @@ ExprResult Sema::BuildCXXDefaultArgExpr(SourceLocation CallLoc,
Init, InitializationContext->Context);
}
+static FieldDecl *FindFieldDeclInstantiationPattern(const ASTContext &Ctx,
+ FieldDecl *Field) {
+ if (FieldDecl *Pattern = Ctx.getInstantiatedFromUnnamedFieldDecl(Field))
+ return Pattern;
+ auto *ParentRD = cast<CXXRecordDecl>(Field->getParent());
+ CXXRecordDecl *ClassPattern = ParentRD->getTemplateInstantiationPattern();
+ DeclContext::lookup_result Lookup =
+ ClassPattern->lookup(Field->getDeclName());
+ auto Rng = llvm::make_filter_range(
+ Lookup, [](auto &&L) { return isa<FieldDecl>(*L); });
+ if (Rng.empty())
+ return nullptr;
+ // FIXME: this breaks clang/test/Modules/pr28812.cpp
+ // assert(std::distance(Rng.begin(), Rng.end()) <= 1
+ // && "Duplicated instantiation pattern for field decl");
+ return cast<FieldDecl>(*Rng.begin());
+}
+
ExprResult Sema::BuildCXXDefaultInitExpr(SourceLocation Loc, FieldDecl *Field) {
assert(Field->hasInClassInitializer());
@@ -5588,15 +5606,8 @@ ExprResult Sema::BuildCXXDefaultInitExpr(SourceLocation Loc, FieldDecl *Field) {
// Maybe we haven't instantiated the in-class initializer. Go check the
// pattern FieldDecl to see if it has one.
if (isTemplateInstantiation(ParentRD->getTemplateSpecializationKind())) {
- CXXRecordDecl *ClassPattern = ParentRD->getTemplateInstantiationPattern();
- DeclContext::lookup_result Lookup =
- ClassPattern->lookup(Field->getDeclName());
-
- FieldDecl *Pattern = nullptr;
- for (auto *L : Lookup) {
- if ((Pattern = dyn_cast<FieldDecl>(L)))
- break;
- }
+ FieldDecl *Pattern =
+ FindFieldDeclInstantiationPattern(getASTContext(), Field);
assert(Pattern && "We must have set the Pattern!");
if (!Pattern->hasInClassInitializer() ||
InstantiateInClassInitializer(Loc, Field, Pattern,
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 3e948232057afe..b7baed49108709 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -1346,7 +1346,7 @@ Decl *TemplateDeclInstantiator::VisitFieldDecl(FieldDecl *D) {
if (Invalid)
Field->setInvalidDecl();
- if (!Field->getDeclName()) {
+ if (!Field->getDeclName() || Field->isPlaceholderVar(SemaRef.getLangOpts())) {
// Keep track of where this decl came from.
SemaRef.Context.setInstantiatedFromUnnamedFieldDecl(Field, D);
}
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp
index d4e392dcc6bcd0..42951807828d0d 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -1539,7 +1539,8 @@ void ASTDeclReader::VisitFieldDecl(FieldDecl *FD) {
else if (Bits & 1)
FD->setBitWidth(Record.readExpr());
- if (!FD->getDeclName()) {
+ if (!FD->getDeclName() ||
+ FD->isPlaceholderVar(Reader.getContext().getLangOpts())) {
if (auto *Tmpl = readDeclAs<FieldDecl>())
Reader.getContext().setInstantiatedFromUnnamedFieldDecl(FD, Tmpl);
}
diff --git a/clang/lib/Serialization/ASTWriterDecl.cpp b/clang/lib/Serialization/ASTWriterDecl.cpp
index b5fe16bf6e787b..14db0c082c937f 100644
--- a/clang/lib/Serialization/ASTWriterDecl.cpp
+++ b/clang/lib/Serialization/ASTWriterDecl.cpp
@@ -1038,7 +1038,7 @@ void ASTDeclWriter::VisitFieldDecl(FieldDecl *D) {
else if (D->BitField)
Record.AddStmt(D->getBitWidth());
- if (!D->getDeclName())
+ if (!D->getDeclName() || D->isPlaceholderVar(Writer.getLangOpts()))
Record.AddDeclRef(Context.getInstantiatedFromUnnamedFieldDecl(D));
if (D->getDeclContext() == D->getLexicalDeclContext() &&
diff --git a/clang/test/SemaCXX/cxx2c-placeholder-vars.cpp b/clang/test/SemaCXX/cxx2c-placeholder-vars.cpp
index 29ca3b5ef3df72..8e428c0ef04279 100644
--- a/clang/test/SemaCXX/cxx2c-placeholder-vars.cpp
+++ b/clang/test/SemaCXX/cxx2c-placeholder-vars.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang -cc1 -fsyntax-only -verify -std=c++2c -Wunused-parameter -Wunused -Wpre-c++26-compat %s
+// RUN: %clang_cc1 -fsyntax-only -verify -ast-dump -std=c++2c -Wunused-parameter -Wunused -Wpre-c++26-compat %s | FileCheck %s
void static_var() {
static int _; // expected-note {{previous definition is here}} \
@@ -254,3 +254,37 @@ namespace Bases {
}
};
}
+
+namespace GH114069 {
+
+template <class T>
+struct A {
+ T _ = 1;
+ T _ = 2;
+ T : 1;
+ T a = 3;
+ T _ = 4;
+};
+
+void f() {
+ [[maybe_unused]] A<int> a;
+}
+
+// CHECK: NamespaceDecl {{.*}} GH114069
+// CHECK: ClassTemplateSpecializationDecl {{.*}} struct A definition
+// CHECK: CXXConstructorDecl {{.*}} implicit used constexpr A 'void () noexcept'
+// CHECK-NEXT: CXXCtorInitializer Field {{.*}} '_' 'int'
+// CHECK-NEXT: CXXDefaultInitExpr {{.*}} 'int' has rewritten init
+// CHECK-NEXT: IntegerLiteral {{.*}} 'int' 1
+// CHECK-NEXT: CXXCtorInitializer Field {{.*}} '_' 'int'
+// CHECK-NEXT: CXXDefaultInitExpr {{.*}} 'int' has rewritten init
+// CHECK-NEXT: IntegerLiteral {{.*}} 'int' 2
+// CHECK-NEXT: CXXCtorInitializer Field {{.*}} 'a' 'int'
+// CHECK-NEXT: CXXDefaultInitExpr {{.*}} 'int' has rewritten init
+// CHECK-NEXT: IntegerLiteral {{.*}} 'int' 3
+// CHECK-NEXT: CXXCtorInitializer Field {{.*}} '_' 'int'
+// CHECK-NEXT: CXXDefaultInitExpr {{.*}} 'int' has rewritten init
+// CHECK-NEXT: IntegerLiteral {{.*}} 'int' 4
+// CHECK-NEXT: CompoundStmt {{.*}}
+
+}
More information about the cfe-commits
mailing list