From 2eaf6f973cacd7f24691b72a9c6a3ee2a3d1a60b Mon Sep 17 00:00:00 2001 From: Sam McCall Date: Tue, 20 Sep 2022 21:39:47 +0200 Subject: [PATCH] [AST] Preserve more structure in UsingEnumDecl node. - store NestedNameSpecifier & Loc for the qualifiers This information was entirely missing from the AST. - expose the location information for qualifier/identifier/typedefs as typeloc This allows many traversals/astmatchers etc to handle these generically along with other references. The decl vs type split can help preserve typedef sugar when https://github.com/llvm/llvm-project/issues/57659 is resolved. - fix the SourceRange of UsingEnumDecl to include 'using'. Fixes https://github.com/clangd/clangd/issues/1283 Differential Revision: https://reviews.llvm.org/D134303 --- clang-tools-extra/clangd/FindTarget.cpp | 6 ++++ .../clangd/unittests/FindTargetTests.cpp | 9 ++++++ .../clangd/unittests/SelectionTests.cpp | 28 ++++++++++++++++++ .../clangd/unittests/SemanticHighlightingTests.cpp | 8 +++++ clang/include/clang/AST/DeclCXX.h | 34 +++++++++++++++------- clang/include/clang/AST/RecursiveASTVisitor.h | 3 +- clang/include/clang/Sema/Sema.h | 3 +- clang/lib/AST/ASTImporter.cpp | 5 ++-- clang/lib/AST/DeclCXX.cpp | 17 +++++++---- clang/lib/Sema/SemaDeclCXX.cpp | 23 +++++++++++---- clang/lib/Sema/SemaTemplateInstantiateDecl.cpp | 4 ++- clang/lib/Serialization/ASTReaderDecl.cpp | 2 +- clang/lib/Serialization/ASTWriterDecl.cpp | 2 +- clang/test/AST/ast-dump-using-enum.cpp | 2 +- .../RecursiveASTVisitorTestTypeLocVisitor.cpp | 8 +++++ 15 files changed, 124 insertions(+), 30 deletions(-) diff --git a/clang-tools-extra/clangd/FindTarget.cpp b/clang-tools-extra/clangd/FindTarget.cpp index dea5cb7..f76bede 100644 --- a/clang-tools-extra/clangd/FindTarget.cpp +++ b/clang-tools-extra/clangd/FindTarget.cpp @@ -629,6 +629,12 @@ llvm::SmallVector refInDecl(const Decl *D, DeclRelation::Underlying, Resolver)}); } + void VisitUsingEnumDecl(const UsingEnumDecl *D) { + // "using enum ns::E" is a non-declaration reference. + // The reference is covered by the embedded typeloc. + // Don't use the default VisitNamedDecl, which would report a declaration. + } + void VisitNamespaceAliasDecl(const NamespaceAliasDecl *D) { // For namespace alias, "namespace Foo = Target;", we add two references. // Add a declaration reference for Foo. diff --git a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp index fd25e80..57838ea 100644 --- a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp +++ b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp @@ -1267,6 +1267,15 @@ TEST_F(FindExplicitReferencesTest, All) { )cpp", "0: targets = {ns}\n" "1: targets = {ns::global}, qualifier = 'ns::'\n"}, + // Using enum declarations. + {R"cpp( + namespace ns { enum class A {}; } + void foo() { + using enum $0^ns::$1^A; + } + )cpp", + "0: targets = {ns}\n" + "1: targets = {ns::A}, qualifier = 'ns::'\n"}, // Simple types. {R"cpp( struct Struct { int a; }; diff --git a/clang-tools-extra/clangd/unittests/SelectionTests.cpp b/clang-tools-extra/clangd/unittests/SelectionTests.cpp index 239566a..bd3df24 100644 --- a/clang-tools-extra/clangd/unittests/SelectionTests.cpp +++ b/clang-tools-extra/clangd/unittests/SelectionTests.cpp @@ -531,6 +531,33 @@ TEST(SelectionTest, CommonAncestor) { void func() { [[__^func__]]; } )cpp", "PredefinedExpr"}, + + // using enum + {R"cpp( + namespace ns { enum class A {}; }; + using enum ns::[[^A]]; + )cpp", + "EnumTypeLoc"}, + {R"cpp( + namespace ns { enum class A {}; using B = A; }; + using enum ns::[[^B]]; + )cpp", + "TypedefTypeLoc"}, + {R"cpp( + namespace ns { enum class A {}; }; + using enum [[^ns::]]A; + )cpp", + "NestedNameSpecifierLoc"}, + {R"cpp( + namespace ns { enum class A {}; }; + [[using ^enum ns::A]]; + )cpp", + "UsingEnumDecl"}, + {R"cpp( + namespace ns { enum class A {}; }; + [[^using enum ns::A]]; + )cpp", + "UsingEnumDecl"}, }; for (const Case &C : Cases) { @@ -541,6 +568,7 @@ TEST(SelectionTest, CommonAncestor) { TU.Code = std::string(Test.code()); TU.ExtraArgs.push_back("-xobjective-c++"); + TU.ExtraArgs.push_back("-std=c++20"); auto AST = TU.build(); auto T = makeSelectionTree(C.Code, AST); diff --git a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp index 32f0f58..e851bea 100644 --- a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp +++ b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp @@ -828,6 +828,14 @@ sizeof...($TemplateParameter[[Elements]]); typedef int $Primitive_decl[[MyTypedef]]; enum $Enum_decl[[MyEnum]] : $Primitive[[MyTypedef]] {}; )cpp", + // Using enum + R"cpp( + enum class $Enum_decl[[Color]] { $EnumConstant_decl_readonly[[Black]] }; + namespace $Namespace_decl[[ns]] { + using enum $Enum[[Color]]; + $Enum[[Color]] $Variable_decl[[ModelT]] = $EnumConstant[[Black]]; + } + )cpp", // Issue 1096 R"cpp( void $Function_decl[[Foo]](); diff --git a/clang/include/clang/AST/DeclCXX.h b/clang/include/clang/AST/DeclCXX.h index 89c7fb3..126d511 100644 --- a/clang/include/clang/AST/DeclCXX.h +++ b/clang/include/clang/AST/DeclCXX.h @@ -3614,17 +3614,15 @@ public: class UsingEnumDecl : public BaseUsingDecl, public Mergeable { /// The source location of the 'using' keyword itself. SourceLocation UsingLocation; - - /// Location of the 'enum' keyword. + /// The source location of the 'enum' keyword. SourceLocation EnumLocation; - - /// The enum - EnumDecl *Enum; + /// 'qual::SomeEnum' as an EnumType, possibly with Elaborated/Typedef sugar. + TypeSourceInfo *EnumType; UsingEnumDecl(DeclContext *DC, DeclarationName DN, SourceLocation UL, - SourceLocation EL, SourceLocation NL, EnumDecl *ED) - : BaseUsingDecl(UsingEnum, DC, NL, DN), UsingLocation(UL), - EnumLocation(EL), Enum(ED) {} + SourceLocation EL, SourceLocation NL, TypeSourceInfo *EnumType) + : BaseUsingDecl(UsingEnum, DC, NL, DN), UsingLocation(UL), EnumLocation(EL), + EnumType(EnumType){} void anchor() override; @@ -3639,13 +3637,29 @@ public: /// The source location of the 'enum' keyword. SourceLocation getEnumLoc() const { return EnumLocation; } void setEnumLoc(SourceLocation L) { EnumLocation = L; } + NestedNameSpecifier *getQualifier() const { + return getQualifierLoc().getNestedNameSpecifier(); + } + NestedNameSpecifierLoc getQualifierLoc() const { + if (auto ETL = EnumType->getTypeLoc().getAs()) + return ETL.getQualifierLoc(); + return NestedNameSpecifierLoc(); + } + // Returns the "qualifier::Name" part as a TypeLoc. + TypeLoc getEnumTypeLoc() const { + return EnumType->getTypeLoc(); + } + TypeSourceInfo *getEnumType() const { + return EnumType; + } + void setEnumType(TypeSourceInfo *TSI) { EnumType = TSI; } public: - EnumDecl *getEnumDecl() const { return Enum; } + EnumDecl *getEnumDecl() const { return cast(EnumType->getType()->getAsTagDecl()); } static UsingEnumDecl *Create(ASTContext &C, DeclContext *DC, SourceLocation UsingL, SourceLocation EnumL, - SourceLocation NameL, EnumDecl *ED); + SourceLocation NameL, TypeSourceInfo *EnumType); static UsingEnumDecl *CreateDeserialized(ASTContext &C, unsigned ID); diff --git a/clang/include/clang/AST/RecursiveASTVisitor.h b/clang/include/clang/AST/RecursiveASTVisitor.h index 9f5d1be..b5c24e1 100644 --- a/clang/include/clang/AST/RecursiveASTVisitor.h +++ b/clang/include/clang/AST/RecursiveASTVisitor.h @@ -1724,7 +1724,8 @@ DEF_TRAVERSE_DECL(UsingDecl, { TRY_TO(TraverseDeclarationNameInfo(D->getNameInfo())); }) -DEF_TRAVERSE_DECL(UsingEnumDecl, {}) +DEF_TRAVERSE_DECL(UsingEnumDecl, + { TRY_TO(TraverseTypeLoc(D->getEnumTypeLoc())); }) DEF_TRAVERSE_DECL(UsingPackDecl, {}) diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index 405f84f..a7f3ac1 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -6119,7 +6119,8 @@ public: NamedDecl *BuildUsingEnumDeclaration(Scope *S, AccessSpecifier AS, SourceLocation UsingLoc, SourceLocation EnumLoc, - SourceLocation NameLoc, EnumDecl *ED); + SourceLocation NameLoc, + TypeSourceInfo *EnumType, EnumDecl *ED); NamedDecl *BuildUsingPackDecl(NamedDecl *InstantiatedFrom, ArrayRef Expansions); diff --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp index 65aff71..2406a69 100644 --- a/clang/lib/AST/ASTImporter.cpp +++ b/clang/lib/AST/ASTImporter.cpp @@ -4870,13 +4870,14 @@ ExpectedDecl ASTNodeImporter::VisitUsingEnumDecl(UsingEnumDecl *D) { Error Err = Error::success(); auto ToUsingLoc = importChecked(Err, D->getUsingLoc()); auto ToEnumLoc = importChecked(Err, D->getEnumLoc()); - auto ToEnumDecl = importChecked(Err, D->getEnumDecl()); + auto ToNameLoc = importChecked(Err, D->getLocation()); + auto *ToEnumType = importChecked(Err, D->getEnumType()); if (Err) return std::move(Err); UsingEnumDecl *ToUsingEnum; if (GetImportedOrCreateDecl(ToUsingEnum, D, Importer.getToContext(), DC, - ToUsingLoc, ToEnumLoc, Loc, ToEnumDecl)) + ToUsingLoc, ToEnumLoc, ToNameLoc, ToEnumType)) return ToUsingEnum; ToUsingEnum->setLexicalDeclContext(LexicalDC); diff --git a/clang/lib/AST/DeclCXX.cpp b/clang/lib/AST/DeclCXX.cpp index 4e259df..40a7ae8 100644 --- a/clang/lib/AST/DeclCXX.cpp +++ b/clang/lib/AST/DeclCXX.cpp @@ -3091,18 +3091,23 @@ SourceRange UsingDecl::getSourceRange() const { void UsingEnumDecl::anchor() {} UsingEnumDecl *UsingEnumDecl::Create(ASTContext &C, DeclContext *DC, - SourceLocation UL, SourceLocation EL, - SourceLocation NL, EnumDecl *Enum) { - return new (C, DC) UsingEnumDecl(DC, Enum->getDeclName(), UL, EL, NL, Enum); + SourceLocation UL, + SourceLocation EL, + SourceLocation NL, + TypeSourceInfo *EnumType) { + assert(isa(EnumType->getType()->getAsTagDecl())); + return new (C, DC) + UsingEnumDecl(DC, EnumType->getType()->getAsTagDecl()->getDeclName(), UL, EL, NL, EnumType); } UsingEnumDecl *UsingEnumDecl::CreateDeserialized(ASTContext &C, unsigned ID) { - return new (C, ID) UsingEnumDecl(nullptr, DeclarationName(), SourceLocation(), - SourceLocation(), SourceLocation(), nullptr); + return new (C, ID) + UsingEnumDecl(nullptr, DeclarationName(), SourceLocation(), + SourceLocation(), SourceLocation(), nullptr); } SourceRange UsingEnumDecl::getSourceRange() const { - return SourceRange(EnumLocation, getLocation()); + return SourceRange(UsingLocation, EnumType->getTypeLoc().getEndLoc()); } void UsingPackDecl::anchor() {} diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index bed2819..27a61c2 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -11871,8 +11871,14 @@ Decl *Sema::ActOnUsingEnumDeclaration(Scope *S, AccessSpecifier AS, SourceLocation IdentLoc, IdentifierInfo &II, CXXScopeSpec *SS) { assert(!SS->isInvalid() && "ScopeSpec is invalid"); - ParsedType TypeRep = getTypeName(II, IdentLoc, S, SS); - if (!TypeRep) { + TypeSourceInfo *TSI = nullptr; + QualType EnumTy = GetTypeFromParser( + getTypeName(II, IdentLoc, S, SS, /*isClassName=*/false, + /*HasTrailingDot=*/false, + /*ObjectType=*/nullptr, /*IsCtorOrDtorName=*/false, + /*WantNontrivialTypeSourceInfo=*/true), + &TSI); + if (EnumTy.isNull()) { Diag(IdentLoc, SS && isDependentScopeSpecifier(*SS) ? diag::err_using_enum_is_dependent : diag::err_unknown_typename) @@ -11881,17 +11887,21 @@ Decl *Sema::ActOnUsingEnumDeclaration(Scope *S, AccessSpecifier AS, return nullptr; } - auto *Enum = dyn_cast_if_present(TypeRep.get()->getAsTagDecl()); + auto *Enum = dyn_cast_if_present(EnumTy->getAsTagDecl()); if (!Enum) { - Diag(IdentLoc, diag::err_using_enum_not_enum) << TypeRep.get(); + Diag(IdentLoc, diag::err_using_enum_not_enum) << EnumTy; return nullptr; } if (auto *Def = Enum->getDefinition()) Enum = Def; + if (TSI == nullptr) + TSI = Context.getTrivialTypeSourceInfo(EnumTy, IdentLoc); + auto *UD = - BuildUsingEnumDeclaration(S, AS, UsingLoc, EnumLoc, IdentLoc, Enum); + BuildUsingEnumDeclaration(S, AS, UsingLoc, EnumLoc, IdentLoc, TSI, Enum); + if (UD) PushOnScopeChains(UD, S, /*AddToContext*/ false); @@ -12583,6 +12593,7 @@ NamedDecl *Sema::BuildUsingEnumDeclaration(Scope *S, AccessSpecifier AS, SourceLocation UsingLoc, SourceLocation EnumLoc, SourceLocation NameLoc, + TypeSourceInfo *EnumType, EnumDecl *ED) { bool Invalid = false; @@ -12609,7 +12620,7 @@ NamedDecl *Sema::BuildUsingEnumDeclaration(Scope *S, AccessSpecifier AS, Invalid = true; UsingEnumDecl *UD = UsingEnumDecl::Create(Context, CurContext, UsingLoc, - EnumLoc, NameLoc, ED); + EnumLoc, NameLoc, EnumType); UD->setAccess(AS); CurContext->addDecl(UD); diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp index dfae702..e6650fb 100644 --- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp +++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp @@ -3294,9 +3294,11 @@ Decl *TemplateDeclInstantiator::VisitUsingEnumDecl(UsingEnumDecl *D) { if (SemaRef.RequireCompleteEnumDecl(EnumD, EnumD->getLocation())) return nullptr; + TypeSourceInfo *TSI = SemaRef.SubstType(D->getEnumType(), TemplateArgs, + D->getLocation(), D->getDeclName()); UsingEnumDecl *NewUD = UsingEnumDecl::Create(SemaRef.Context, Owner, D->getUsingLoc(), - D->getEnumLoc(), D->getLocation(), EnumD); + D->getEnumLoc(), D->getLocation(), TSI); SemaRef.Context.setInstantiatedFromUsingEnumDecl(NewUD, D); NewUD->setAccess(D->getAccess()); diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp index b7abcfa..e53f729 100644 --- a/clang/lib/Serialization/ASTReaderDecl.cpp +++ b/clang/lib/Serialization/ASTReaderDecl.cpp @@ -1773,7 +1773,7 @@ void ASTDeclReader::VisitUsingEnumDecl(UsingEnumDecl *D) { VisitNamedDecl(D); D->setUsingLoc(readSourceLocation()); D->setEnumLoc(readSourceLocation()); - D->Enum = readDeclAs(); + D->setEnumType(Record.readTypeSourceInfo()); D->FirstUsingShadow.setPointer(readDeclAs()); if (auto *Pattern = readDeclAs()) Reader.getContext().setInstantiatedFromUsingEnumDecl(D, Pattern); diff --git a/clang/lib/Serialization/ASTWriterDecl.cpp b/clang/lib/Serialization/ASTWriterDecl.cpp index f9858a5..82b06f3 100644 --- a/clang/lib/Serialization/ASTWriterDecl.cpp +++ b/clang/lib/Serialization/ASTWriterDecl.cpp @@ -1294,7 +1294,7 @@ void ASTDeclWriter::VisitUsingEnumDecl(UsingEnumDecl *D) { VisitNamedDecl(D); Record.AddSourceLocation(D->getUsingLoc()); Record.AddSourceLocation(D->getEnumLoc()); - Record.AddDeclRef(D->getEnumDecl()); + Record.AddTypeSourceInfo(D->getEnumType()); Record.AddDeclRef(D->FirstUsingShadow.getPointer()); Record.AddDeclRef(Context.getInstantiatedFromUsingEnumDecl(D)); Code = serialization::DECL_USING_ENUM; diff --git a/clang/test/AST/ast-dump-using-enum.cpp b/clang/test/AST/ast-dump-using-enum.cpp index 3f848ca..26cf9e3 100644 --- a/clang/test/AST/ast-dump-using-enum.cpp +++ b/clang/test/AST/ast-dump-using-enum.cpp @@ -21,7 +21,7 @@ using enum Bob::Foo; // CHECK-NEXT: `-EnumConstantDecl {{.*}} Foo_b 'Bob::Foo' // CHECK-LABEL: Dumping Foo: -// CHECK-NEXT: UsingEnumDecl {{.*}} Enum {{.*}} 'Foo' +// CHECK-NEXT: UsingEnumDecl {{.*}} <{{.*}}:16:1, col:17> {{.*}} Enum {{.*}} 'Foo' // CHECK-LABEL: Dumping Foo_a: // CHECK-NEXT: UsingShadowDecl {{.*}} implicit EnumConstant {{.*}} 'Foo_a' 'Bob::Foo' diff --git a/clang/unittests/Tooling/RecursiveASTVisitorTestTypeLocVisitor.cpp b/clang/unittests/Tooling/RecursiveASTVisitorTestTypeLocVisitor.cpp index 6c6670c..a211862 100644 --- a/clang/unittests/Tooling/RecursiveASTVisitorTestTypeLocVisitor.cpp +++ b/clang/unittests/Tooling/RecursiveASTVisitorTestTypeLocVisitor.cpp @@ -88,4 +88,12 @@ TEST(RecursiveASTVisitor, VisitInvalidType) { TypeLocVisitor::Lang_C)); } +TEST(RecursiveASTVisitor, VisitsUsingEnumType) { + TypeLocVisitor Visitor; + Visitor.ExpectMatch("::A", 2, 12); + EXPECT_TRUE(Visitor.runOver("enum class A {}; \n" + "using enum ::A;\n", + TypeLocVisitor::Lang_CXX2a)); +} + } // end anonymous namespace -- 2.7.4