[clang] 6a5e08c - [AST] injected-class-name is not a redecl, even in template specializations
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Tue Nov 2 06:38:01 PDT 2021
Author: Sam McCall
Date: 2021-11-02T14:37:45+01:00
New Revision: 6a5e08cc4a5c64de08a277dd2e0a862120c5fc28
URL: https://github.com/llvm/llvm-project/commit/6a5e08cc4a5c64de08a277dd2e0a862120c5fc28
DIFF: https://github.com/llvm/llvm-project/commit/6a5e08cc4a5c64de08a277dd2e0a862120c5fc28.diff
LOG: [AST] injected-class-name is not a redecl, even in template specializations
Back in the mists of time, the CXXRecordDecl for the injected-class-name was
a redecl of the outer class itself.
This got changed in 470c454a6176ef31474553e408c90f5ee630df89, but only for plain
classes: class template instantation was still detecting the injected-class-name
in the template body and marking its instantiation as a redecl.
This causes some subtle inconsistent behavior between the two, e.g.
hasDefinition() returns true for Foo<int>::Foo but false for Bar::Bar.
This is the root cause of PR51912.
Differential Revision: https://reviews.llvm.org/D112765
Added:
Modified:
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp
clang/include/clang/AST/RecursiveASTVisitor.h
clang/lib/AST/ASTDumper.cpp
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
clang/test/AST/ast-dump-decl.cpp
clang/test/AST/ast-dump-openmp-begin-declare-variant_reference.cpp
clang/test/AST/ast-dump-openmp-begin-declare-variant_template_3.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp
index 3fe392e05a95f..51535f89ac43d 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp
@@ -231,9 +231,8 @@ struct DerivedFromTemplateVirtualBaseStruct : T {
DerivedFromTemplateVirtualBaseStruct<PublicVirtualBaseStruct> InstantiationWithPublicVirtualBaseStruct;
// Derived from template, base has *not* virtual dtor
-// CHECK-MESSAGES: :[[@LINE+8]]:8: warning: destructor of 'DerivedFromTemplateNonVirtualBaseStruct' is public and non-virtual [cppcoreguidelines-virtual-class-destructor]
-// CHECK-MESSAGES: :[[@LINE+7]]:8: note: make it public and virtual
-// CHECK-MESSAGES: :[[@LINE+6]]:8: warning: destructor of 'DerivedFromTemplateNonVirtualBaseStruct<PublicNonVirtualBaseStruct>' is public and non-virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+7]]:8: warning: destructor of 'DerivedFromTemplateNonVirtualBaseStruct<PublicNonVirtualBaseStruct>' is public and non-virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+6]]:8: note: make it public and virtual
// CHECK-FIXES: struct DerivedFromTemplateNonVirtualBaseStruct : T {
// CHECK-FIXES-NEXT: virtual ~DerivedFromTemplateNonVirtualBaseStruct() = default;
// CHECK-FIXES-NEXT: virtual void foo();
@@ -256,9 +255,8 @@ using DerivedFromTemplateVirtualBaseStruct2Typedef = DerivedFromTemplateVirtualB
DerivedFromTemplateVirtualBaseStruct2Typedef InstantiationWithPublicVirtualBaseStruct2;
// Derived from template, base has *not* virtual dtor, to be used in a typedef
-// CHECK-MESSAGES: :[[@LINE+8]]:8: warning: destructor of 'DerivedFromTemplateNonVirtualBaseStruct2' is public and non-virtual [cppcoreguidelines-virtual-class-destructor]
-// CHECK-MESSAGES: :[[@LINE+7]]:8: note: make it public and virtual
-// CHECK-MESSAGES: :[[@LINE+6]]:8: warning: destructor of 'DerivedFromTemplateNonVirtualBaseStruct2<PublicNonVirtualBaseStruct>' is public and non-virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+7]]:8: warning: destructor of 'DerivedFromTemplateNonVirtualBaseStruct2<PublicNonVirtualBaseStruct>' is public and non-virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+6]]:8: note: make it public and virtual
// CHECK-FIXES: struct DerivedFromTemplateNonVirtualBaseStruct2 : T {
// CHECK-FIXES-NEXT: virtual ~DerivedFromTemplateNonVirtualBaseStruct2() = default;
// CHECK-FIXES-NEXT: virtual void foo();
diff --git a/clang/include/clang/AST/RecursiveASTVisitor.h b/clang/include/clang/AST/RecursiveASTVisitor.h
index 74c49546c00bc..fe26a1de9f85b 100644
--- a/clang/include/clang/AST/RecursiveASTVisitor.h
+++ b/clang/include/clang/AST/RecursiveASTVisitor.h
@@ -1681,10 +1681,7 @@ bool RecursiveASTVisitor<Derived>::TraverseTemplateInstantiations(
ClassTemplateDecl *D) {
for (auto *SD : D->specializations()) {
for (auto *RD : SD->redecls()) {
- // We don't want to visit injected-class-names in this traversal.
- if (cast<CXXRecordDecl>(RD)->isInjectedClassName())
- continue;
-
+ assert(!cast<CXXRecordDecl>(RD)->isInjectedClassName());
switch (
cast<ClassTemplateSpecializationDecl>(RD)->getSpecializationKind()) {
// Visit the implicit instantiations with the requested pattern.
diff --git a/clang/lib/AST/ASTDumper.cpp b/clang/lib/AST/ASTDumper.cpp
index 3d368a0a7b63e..c6df61f79e2e8 100644
--- a/clang/lib/AST/ASTDumper.cpp
+++ b/clang/lib/AST/ASTDumper.cpp
@@ -90,15 +90,7 @@ void ASTDumper::dumpTemplateDeclSpecialization(const SpecializationDecl *D,
// FIXME: The redecls() range sometimes has elements of a less-specific
// type. (In particular, ClassTemplateSpecializationDecl::redecls() gives
// us TagDecls, and should give CXXRecordDecls).
- auto *Redecl = dyn_cast<SpecializationDecl>(RedeclWithBadType);
- if (!Redecl) {
- // Found the injected-class-name for a class template. This will be dumped
- // as part of its surrounding class so we don't need to dump it here.
- assert(isa<CXXRecordDecl>(RedeclWithBadType) &&
- "expected an injected-class-name");
- continue;
- }
-
+ auto *Redecl = cast<SpecializationDecl>(RedeclWithBadType);
switch (Redecl->getTemplateSpecializationKind()) {
case TSK_ExplicitInstantiationDeclaration:
case TSK_ExplicitInstantiationDefinition:
diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 7142e6ae56bb3..8c433a0e8cca7 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -1818,9 +1818,7 @@ TemplateDeclInstantiator::VisitFunctionTemplateDecl(FunctionTemplateDecl *D) {
Decl *TemplateDeclInstantiator::VisitCXXRecordDecl(CXXRecordDecl *D) {
CXXRecordDecl *PrevDecl = nullptr;
- if (D->isInjectedClassName())
- PrevDecl = cast<CXXRecordDecl>(Owner);
- else if (CXXRecordDecl *PatternPrev = getPreviousDeclForInstantiation(D)) {
+ if (CXXRecordDecl *PatternPrev = getPreviousDeclForInstantiation(D)) {
NamedDecl *Prev = SemaRef.FindInstantiatedDecl(D->getLocation(),
PatternPrev,
TemplateArgs);
@@ -1829,6 +1827,7 @@ Decl *TemplateDeclInstantiator::VisitCXXRecordDecl(CXXRecordDecl *D) {
}
CXXRecordDecl *Record = nullptr;
+ bool IsInjectedClassName = D->isInjectedClassName();
if (D->isLambda())
Record = CXXRecordDecl::CreateLambda(
SemaRef.Context, Owner, D->getLambdaTypeInfo(), D->getLocation(),
@@ -1837,7 +1836,11 @@ Decl *TemplateDeclInstantiator::VisitCXXRecordDecl(CXXRecordDecl *D) {
else
Record = CXXRecordDecl::Create(SemaRef.Context, D->getTagKind(), Owner,
D->getBeginLoc(), D->getLocation(),
- D->getIdentifier(), PrevDecl);
+ D->getIdentifier(), PrevDecl,
+ /*DelayTypeCreation=*/IsInjectedClassName);
+ // Link the type of the injected-class-name to that of the outer class.
+ if (IsInjectedClassName)
+ (void)SemaRef.Context.getTypeDeclType(Record, cast<CXXRecordDecl>(Owner));
// Substitute the nested name specifier, if any.
if (SubstQualifier(D, Record))
@@ -1852,7 +1855,7 @@ Decl *TemplateDeclInstantiator::VisitCXXRecordDecl(CXXRecordDecl *D) {
// specifier. Remove once this area of the code gets sorted out.
if (D->getAccess() != AS_none)
Record->setAccess(D->getAccess());
- if (!D->isInjectedClassName())
+ if (!IsInjectedClassName)
Record->setInstantiationOfMemberClass(D, TSK_ImplicitInstantiation);
// If the original function was part of a friend declaration,
@@ -1905,6 +1908,9 @@ Decl *TemplateDeclInstantiator::VisitCXXRecordDecl(CXXRecordDecl *D) {
SemaRef.DiagnoseUnusedNestedTypedefs(Record);
+ if (IsInjectedClassName)
+ assert(Record->isInjectedClassName() && "Broken injected-class-name");
+
return Record;
}
diff --git a/clang/test/AST/ast-dump-decl.cpp b/clang/test/AST/ast-dump-decl.cpp
index 0ff947ae3b483..acbe5207f174b 100644
--- a/clang/test/AST/ast-dump-decl.cpp
+++ b/clang/test/AST/ast-dump-decl.cpp
@@ -321,7 +321,7 @@ namespace testClassTemplateDecl {
// CHECK-NEXT: | |-TemplateArgument type 'testClassTemplateDecl::A'
// CHECK-NEXT: | | `-RecordType 0{{.+}} 'testClassTemplateDecl::A'
// CHECK-NEXT: | | `-CXXRecord 0x{{.+}} 'A'
-// CHECK-NEXT: | |-CXXRecordDecl 0x{{.+}} prev 0x{{.+}} <col:24, col:30> col:30 implicit class TestClassTemplate
+// CHECK-NEXT: | |-CXXRecordDecl 0x{{.+}} <col:24, col:30> col:30 implicit class TestClassTemplate
// CHECK-NEXT: | |-AccessSpecDecl 0x{{.+}} <line:[[@LINE-67]]:3, col:9> col:3 public
// CHECK-NEXT: | |-CXXConstructorDecl 0x{{.+}} <line:[[@LINE-67]]:5, col:23> col:5 used TestClassTemplate 'void ()'
// CHECK-NEXT: | |-CXXDestructorDecl 0x{{.+}} <line:[[@LINE-67]]:5, col:24> col:5 used ~TestClassTemplate 'void () noexcept'
@@ -358,7 +358,7 @@ namespace testClassTemplateDecl {
// CHECK-NEXT: |-TemplateArgument type 'testClassTemplateDecl::C'
// CHECK-NEXT: | `-RecordType 0{{.+}} 'testClassTemplateDecl::C'
// CHECK-NEXT: | `-CXXRecord 0x{{.+}} 'C'
-// CHECK-NEXT: |-CXXRecordDecl 0x{{.+}} prev 0x{{.+}} <line:[[@LINE-104]]:24, col:30> col:30 implicit class TestClassTemplate
+// CHECK-NEXT: |-CXXRecordDecl 0x{{.+}} <line:[[@LINE-104]]:24, col:30> col:30 implicit class TestClassTemplate
// CHECK-NEXT: |-AccessSpecDecl 0x{{.+}} <line:[[@LINE-104]]:3, col:9> col:3 public
// CHECK-NEXT: |-CXXConstructorDecl 0x{{.+}} <line:[[@LINE-104]]:5, col:23> col:5 TestClassTemplate 'void ()'
// CHECK-NEXT: |-CXXDestructorDecl 0x{{.+}} <line:[[@LINE-104]]:5, col:24> col:5 ~TestClassTemplate 'void ()' noexcept-unevaluated 0x{{.+}}
@@ -376,7 +376,7 @@ namespace testClassTemplateDecl {
// CHECK-NEXT: |-TemplateArgument type 'testClassTemplateDecl::D'
// CHECK-NEXT: | `-RecordType 0{{.+}} 'testClassTemplateDecl::D'
// CHECK-NEXT: | `-CXXRecord 0x{{.+}} 'D'
-// CHECK-NEXT: |-CXXRecordDecl 0x{{.+}} prev 0x{{.+}} <line:[[@LINE-122]]:24, col:30> col:30 implicit class TestClassTemplate
+// CHECK-NEXT: |-CXXRecordDecl 0x{{.+}} <line:[[@LINE-122]]:24, col:30> col:30 implicit class TestClassTemplate
// CHECK-NEXT: |-AccessSpecDecl 0x{{.+}} <line:[[@LINE-122]]:3, col:9> col:3 public
// CHECK-NEXT: |-CXXConstructorDecl 0x{{.+}} <line:[[@LINE-122]]:5, col:23> col:5 TestClassTemplate 'void ()'
// CHECK-NEXT: |-CXXDestructorDecl 0x{{.+}} <line:[[@LINE-122]]:5, col:24> col:5 ~TestClassTemplate 'void ()' noexcept-unevaluated 0x{{.+}}
@@ -519,7 +519,7 @@ namespace testCanonicalTemplate {
// CHECK-NEXT: |-TemplateArgument type 'testCanonicalTemplate::A'
// CHECK-NEXT: | `-RecordType 0x{{.+}} 'testCanonicalTemplate::A'
// CHECK-NEXT: | `-CXXRecord 0x{{.+}} 'A'
- // CHECK-NEXT: |-CXXRecordDecl 0x{{.+}} prev 0x{{.+}} <col:25, col:31> col:31 implicit class TestClassTemplate
+ // CHECK-NEXT: |-CXXRecordDecl 0x{{.+}} <col:25, col:31> col:31 implicit class TestClassTemplate
// CHECK-NEXT: |-FriendDecl 0x{{.+}} <line:[[@LINE-30]]:5, col:40> col:40
// CHECK-NEXT: | `-ClassTemplateDecl 0x{{.+}} parent 0x{{.+}} prev 0x{{.+}} <col:5, col:40> col:40 TestClassTemplate
// CHECK-NEXT: | |-TemplateTypeParmDecl 0x{{.+}} <col:14, col:23> col:23 typename depth 0 index 0 T2
@@ -552,7 +552,7 @@ namespace testCanonicalTemplate {
// CHECK-NEXT: |-TemplateArgument type 'testCanonicalTemplate::A'
// CHECK-NEXT: | `-RecordType 0x{{.+}} 'testCanonicalTemplate::A'
// CHECK-NEXT: | `-CXXRecord 0x{{.+}} 'A'
- // CHECK-NEXT: |-CXXRecordDecl 0x{{.+}} prev 0x{{.+}} <col:25, col:31> col:31 implicit class TestClassTemplate2
+ // CHECK-NEXT: |-CXXRecordDecl 0x{{.+}} <col:25, col:31> col:31 implicit class TestClassTemplate2
// CHECK-NEXT: |-CXXConstructorDecl 0x{{.+}} <col:31> col:31 implicit used constexpr TestClassTemplate2 'void () noexcept' inline default trivial
// CHECK-NEXT: | `-CompoundStmt 0x{{.+}} <col:31>
// CHECK-NEXT: |-CXXConstructorDecl 0x{{.+}} <col:31> col:31 implicit constexpr TestClassTemplate2 'void (const testCanonicalTemplate::TestClassTemplate2<testCanonicalTemplate::A> &)' inline default trivial noexcept-unevaluated 0x{{.+}}
diff --git a/clang/test/AST/ast-dump-openmp-begin-declare-variant_reference.cpp b/clang/test/AST/ast-dump-openmp-begin-declare-variant_reference.cpp
index 6fa8cdc59bb40..35ec1cbfd8f7e 100644
--- a/clang/test/AST/ast-dump-openmp-begin-declare-variant_reference.cpp
+++ b/clang/test/AST/ast-dump-openmp-begin-declare-variant_reference.cpp
@@ -120,7 +120,7 @@ int test(float &&f, short &&s) {
// CHECK-NEXT: | | |-TemplateArgument type 'float &'
// CHECK-NEXT: | | | `-LValueReferenceType [[ADDR_7:0x[a-z0-9]*]] 'float &'
// CHECK-NEXT: | | | `-BuiltinType [[ADDR_8:0x[a-z0-9]*]] 'float'
-// CHECK-NEXT: | | |-CXXRecordDecl [[ADDR_9:0x[a-z0-9]*]] prev [[ADDR_6]] <col:22, col:29> col:29 implicit struct remove_reference
+// CHECK-NEXT: | | |-CXXRecordDecl [[ADDR_9:0x[a-z0-9]*]] <col:22, col:29> col:29 implicit struct remove_reference
// CHECK-NEXT: | | `-TypedefDecl [[ADDR_10:0x[a-z0-9]*]] <col:55, col:67> col:67 referenced type 'float':'float'
// CHECK-NEXT: | | `-SubstTemplateTypeParmType [[ADDR_11:0x[a-z0-9]*]] 'float' sugar
// CHECK-NEXT: | | |-TemplateTypeParmType [[ADDR_12:0x[a-z0-9]*]] '_Tp' dependent depth 0 index 0
@@ -137,7 +137,7 @@ int test(float &&f, short &&s) {
// CHECK-NEXT: | |-TemplateArgument type 'short &'
// CHECK-NEXT: | | `-LValueReferenceType [[ADDR_15:0x[a-z0-9]*]] 'short &'
// CHECK-NEXT: | | `-BuiltinType [[ADDR_16:0x[a-z0-9]*]] 'short'
-// CHECK-NEXT: | |-CXXRecordDecl [[ADDR_17:0x[a-z0-9]*]] prev [[ADDR_14]] <col:22, col:29> col:29 implicit struct remove_reference
+// CHECK-NEXT: | |-CXXRecordDecl [[ADDR_17:0x[a-z0-9]*]] <col:22, col:29> col:29 implicit struct remove_reference
// CHECK-NEXT: | `-TypedefDecl [[ADDR_18:0x[a-z0-9]*]] <col:55, col:67> col:67 referenced type 'short':'short'
// CHECK-NEXT: | `-SubstTemplateTypeParmType [[ADDR_19:0x[a-z0-9]*]] 'short' sugar
// CHECK-NEXT: | |-TemplateTypeParmType [[ADDR_12]] '_Tp' dependent depth 0 index 0
diff --git a/clang/test/AST/ast-dump-openmp-begin-declare-variant_template_3.cpp b/clang/test/AST/ast-dump-openmp-begin-declare-variant_template_3.cpp
index 153764490c0dd..88c47b213cfbf 100644
--- a/clang/test/AST/ast-dump-openmp-begin-declare-variant_template_3.cpp
+++ b/clang/test/AST/ast-dump-openmp-begin-declare-variant_template_3.cpp
@@ -69,7 +69,7 @@ int test() {
// CHECK-NEXT: | | | `-Destructor simple irrelevant trivial
// CHECK-NEXT: | | |-TemplateArgument type 'int'
// CHECK-NEXT: | | | `-BuiltinType [[ADDR_9:0x[a-z0-9]*]] 'int'
-// CHECK-NEXT: | | |-CXXRecordDecl [[ADDR_10:0x[a-z0-9]*]] prev [[ADDR_8]] <col:23, col:30> col:30 implicit struct S
+// CHECK-NEXT: | | |-CXXRecordDecl [[ADDR_10:0x[a-z0-9]*]] <col:23, col:30> col:30 implicit struct S
// CHECK-NEXT: | | |-CXXConstructorDecl [[ADDR_11:0x[a-z0-9]*]] <line:6:3, col:16> col:3 used S 'void (int, int *)'
// CHECK-NEXT: | | | |-ParmVarDecl [[ADDR_12:0x[a-z0-9]*]] <col:5> col:8 'int'
// CHECK-NEXT: | | | |-ParmVarDecl [[ADDR_13:0x[a-z0-9]*]] <col:10, col:12> col:13 'int *'
More information about the cfe-commits
mailing list