sammccall created this revision.
sammccall added reviewers: hokein, aaron.ballman, whisperity.
Herald added subscribers: rnkovacs, kbarton, nemanjai.
sammccall requested review of this revision.
Herald added projects: clang, clang-tools-extra.
Herald added a subscriber: cfe-commits.
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.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D112765
Files:
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.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
Index: clang/test/AST/ast-dump-openmp-begin-declare-variant_template_3.cpp
===================================================================
--- clang/test/AST/ast-dump-openmp-begin-declare-variant_template_3.cpp
+++ clang/test/AST/ast-dump-openmp-begin-declare-variant_template_3.cpp
@@ -69,7 +69,7 @@
// 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 *'
Index: clang/test/AST/ast-dump-openmp-begin-declare-variant_reference.cpp
===================================================================
--- clang/test/AST/ast-dump-openmp-begin-declare-variant_reference.cpp
+++ clang/test/AST/ast-dump-openmp-begin-declare-variant_reference.cpp
@@ -120,7 +120,7 @@
// 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 @@
// 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
Index: clang/test/AST/ast-dump-decl.cpp
===================================================================
--- clang/test/AST/ast-dump-decl.cpp
+++ clang/test/AST/ast-dump-decl.cpp
@@ -321,7 +321,7 @@
// 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 @@
// 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 @@
// 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 @@
// 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 @@
// 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{{.+}}
Index: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
===================================================================
--- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -1818,9 +1818,7 @@
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 @@
}
CXXRecordDecl *Record = nullptr;
+ bool IsInjectedClassName = D->isInjectedClassName();
if (D->isLambda())
Record = CXXRecordDecl::CreateLambda(
SemaRef.Context, Owner, D->getLambdaTypeInfo(), D->getLocation(),
@@ -1837,7 +1836,10 @@
else
Record = CXXRecordDecl::Create(SemaRef.Context, D->getTagKind(), Owner,
D->getBeginLoc(), D->getLocation(),
- D->getIdentifier(), PrevDecl);
+ D->getIdentifier(), PrevDecl,
+ /*DelayTypeCreation=*/IsInjectedClassName);
+ if (IsInjectedClassName)
+ SemaRef.Context.getTypeDeclType(Record, cast<CXXRecordDecl>(Owner));
// Substitute the nested name specifier, if any.
if (SubstQualifier(D, Record))
@@ -1852,7 +1854,7 @@
// 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 +1907,9 @@
SemaRef.DiagnoseUnusedNestedTypedefs(Record);
+ if (IsInjectedClassName)
+ assert(Record->isInjectedClassName() && "Broken injected-class-name");
+
return Record;
}
Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp
@@ -231,9 +231,8 @@
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 @@
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();
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
