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 *' _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
