diff --git a/clang-tools-extra/clangd/Selection.cpp b/clang-tools-extra/clangd/Selection.cpp --- a/clang-tools-extra/clangd/Selection.cpp +++ b/clang-tools-extra/clangd/Selection.cpp @@ -10,6 +10,7 @@ #include "AST.h" #include "support/Logger.h" #include "support/Trace.h" +#include "clang/AST/ASTConcept.h" #include "clang/AST/ASTTypeTraits.h" #include "clang/AST/Decl.h" #include "clang/AST/DeclCXX.h" @@ -709,6 +710,15 @@ bool TraversePseudoObjectExpr(PseudoObjectExpr *E) { return traverseNode(E, [&] { return TraverseStmt(E->getSyntacticForm()); }); } + bool TraverseTypeConstraint(const TypeConstraint *C) { + if (auto *E = C->getImmediatelyDeclaredConstraint()) { + // Technically this expression is 'implicit' and not traversed by the RAV. + // However, the range is correct, so we visit expression to avoid adding + // an extra kind to 'DynTypeNode' that hold 'TypeConstraint'. + return TraverseStmt(E); + } + return Base::TraverseTypeConstraint(C); + } private: using Base = RecursiveASTVisitor; diff --git a/clang-tools-extra/clangd/unittests/XRefsTests.cpp b/clang-tools-extra/clangd/unittests/XRefsTests.cpp --- a/clang-tools-extra/clangd/unittests/XRefsTests.cpp +++ b/clang-tools-extra/clangd/unittests/XRefsTests.cpp @@ -2085,6 +2085,19 @@ checkFindRefs(Code); } +TEST(FindReferences, ConceptReq) { + constexpr llvm::StringLiteral Code = R"cpp( + template + concept $def[[IsSmal^l]] = sizeof(T) <= 8; + + template + concept IsSmallPtr = requires(T x) { + { *x } -> [[IsSmal^l]]; + }; + )cpp"; + checkFindRefs(Code); +} + TEST(FindReferences, RequiresExprParameters) { constexpr llvm::StringLiteral Code = R"cpp( template diff --git a/clang/include/clang/AST/RecursiveASTVisitor.h b/clang/include/clang/AST/RecursiveASTVisitor.h --- a/clang/include/clang/AST/RecursiveASTVisitor.h +++ b/clang/include/clang/AST/RecursiveASTVisitor.h @@ -13,18 +13,19 @@ #ifndef LLVM_CLANG_AST_RECURSIVEASTVISITOR_H #define LLVM_CLANG_AST_RECURSIVEASTVISITOR_H +#include "clang/AST/ASTConcept.h" #include "clang/AST/Attr.h" #include "clang/AST/Decl.h" -#include "clang/AST/DeclarationName.h" #include "clang/AST/DeclBase.h" #include "clang/AST/DeclCXX.h" #include "clang/AST/DeclFriend.h" #include "clang/AST/DeclObjC.h" #include "clang/AST/DeclOpenMP.h" #include "clang/AST/DeclTemplate.h" +#include "clang/AST/DeclarationName.h" #include "clang/AST/Expr.h" -#include "clang/AST/ExprConcepts.h" #include "clang/AST/ExprCXX.h" +#include "clang/AST/ExprConcepts.h" #include "clang/AST/ExprObjC.h" #include "clang/AST/ExprOpenMP.h" #include "clang/AST/LambdaCapture.h" @@ -319,11 +320,6 @@ bool TraverseSynOrSemInitListExpr(InitListExpr *S, DataRecursionQueue *Queue = nullptr); - /// Recursively visit a reference to a concept with potential arguments. - /// - /// \returns false if the visitation was terminated early, true otherwise. - bool TraverseConceptReference(const ConceptReference &C); - /// Recursively visit an Objective-C protocol reference with location /// information. /// @@ -475,11 +471,21 @@ DEF_TRAVERSE_TMPL_INST(Function) #undef DEF_TRAVERSE_TMPL_INST + bool TraverseTypeConstraint(const TypeConstraint *C); + + bool TraverseConceptRequirement(concepts::Requirement *R); + bool TraverseConceptTypeRequirement(concepts::TypeRequirement *R); + bool TraverseConceptExprRequirement(concepts::ExprRequirement *R); + bool TraverseConceptNestedRequirement(concepts::NestedRequirement *R); + bool dataTraverseNode(Stmt *S, DataRecursionQueue *Queue); private: // These are helper methods used by more than one Traverse* method. bool TraverseTemplateParameterListHelper(TemplateParameterList *TPL); + /// Traverses the qualifier, name and template arguments of a concept + /// reference. + bool TraverseConceptReferenceHelper(const ConceptReference &C); // Traverses template parameter lists of either a DeclaratorDecl or TagDecl. template @@ -511,6 +517,54 @@ bool PostVisitStmt(Stmt *S); }; +template +bool RecursiveASTVisitor::TraverseTypeConstraint( + const TypeConstraint *C) { + if (!getDerived().shouldVisitImplicitCode()) { + TRY_TO(TraverseConceptReferenceHelper(*C)); + return true; + } + if (Expr *IDC = C->getImmediatelyDeclaredConstraint()) { + TRY_TO(TraverseStmt(IDC)); + } else { + // Avoid traversing the ConceptReference in the TypeConstraint + // if we have an immediately-declared-constraint, otherwise + // we'll end up visiting the concept and the arguments in + // the TC twice. + TRY_TO(TraverseConceptReferenceHelper(*C)); + } + return true; +} + +template +bool RecursiveASTVisitor::TraverseConceptRequirement( + concepts::Requirement *R) { + switch (R->getKind()) { + case concepts::Requirement::RK_Type: + return getDerived().TraverseConceptTypeRequirement( + cast(R)); + case concepts::Requirement::RK_Simple: + case concepts::Requirement::RK_Compound: + return getDerived().TraverseConceptExprRequirement( + cast(R)); + case concepts::Requirement::RK_Nested: + return getDerived().TraverseConceptNestedRequirement( + cast(R)); + } +} + +template +bool RecursiveASTVisitor::TraverseConceptReferenceHelper( + const ConceptReference &C) { + TRY_TO(TraverseNestedNameSpecifierLoc(C.getNestedNameSpecifierLoc())); + TRY_TO(TraverseDeclarationNameInfo(C.getConceptNameInfo())); + if (C.hasExplicitTemplateArgs()) + TRY_TO(TraverseTemplateArgumentLocsHelper( + C.getTemplateArgsAsWritten()->getTemplateArgs(), + C.getTemplateArgsAsWritten()->NumTemplateArgs)); + return true; +} + template bool RecursiveASTVisitor::dataTraverseNode(Stmt *S, DataRecursionQueue *Queue) { @@ -530,6 +584,40 @@ #undef DISPATCH_STMT +template +bool RecursiveASTVisitor::TraverseConceptTypeRequirement( + concepts::TypeRequirement *R) { + if (R->isSubstitutionFailure()) + return true; + return getDerived().TraverseTypeLoc(R->getType()->getTypeLoc()); +} + +template +bool RecursiveASTVisitor::TraverseConceptExprRequirement( + concepts::ExprRequirement *R) { + if (!R->isExprSubstitutionFailure()) + TRY_TO(TraverseStmt(R->getExpr())); + auto &RetReq = R->getReturnTypeRequirement(); + if (RetReq.isTypeConstraint()) { + if (getDerived().shouldVisitImplicitCode()) { + TRY_TO(TraverseTemplateParameterListHelper( + RetReq.getTypeConstraintTemplateParameterList())); + } else { + // Template parameter list is implicit, visit constraint directly. + TRY_TO(TraverseTypeConstraint(RetReq.getTypeConstraint())); + } + } + return true; +} + +template +bool RecursiveASTVisitor::TraverseConceptNestedRequirement( + concepts::NestedRequirement *R) { + if (!R->isSubstitutionFailure()) + return getDerived().TraverseStmt(R->getConstraintExpr()); + return true; +} + template bool RecursiveASTVisitor::PostVisitStmt(Stmt *S) { // In pre-order traversal mode, each Traverse##STMT method is responsible for @@ -1007,7 +1095,6 @@ DEF_TRAVERSE_TYPE(AutoType, { TRY_TO(TraverseType(T->getDeducedType())); if (T->isConstrained()) { - TRY_TO(TraverseDecl(T->getTypeConstraintConcept())); TRY_TO(TraverseTemplateArguments(T->getArgs(), T->getNumArgs())); } }) @@ -1838,17 +1925,8 @@ template bool RecursiveASTVisitor::TraverseTemplateTypeParamDeclConstraints( const TemplateTypeParmDecl *D) { - if (const auto *TC = D->getTypeConstraint()) { - if (Expr *IDC = TC->getImmediatelyDeclaredConstraint()) { - TRY_TO(TraverseStmt(IDC)); - } else { - // Avoid traversing the ConceptReference in the TypeCosntraint - // if we have an immediately-declared-constraint, otherwise - // we'll end up visiting the concept and the arguments in - // the TC twice. - TRY_TO(TraverseConceptReference(*TC)); - } - } + if (const auto *TC = D->getTypeConstraint()) + TRY_TO(TraverseTypeConstraint(TC)); return true; } @@ -2435,18 +2513,6 @@ return true; } -template -bool RecursiveASTVisitor::TraverseConceptReference( - const ConceptReference &C) { - TRY_TO(TraverseNestedNameSpecifierLoc(C.getNestedNameSpecifierLoc())); - TRY_TO(TraverseDeclarationNameInfo(C.getConceptNameInfo())); - if (C.hasExplicitTemplateArgs()) - TRY_TO(TraverseTemplateArgumentLocsHelper( - C.getTemplateArgsAsWritten()->getTemplateArgs(), - C.getTemplateArgsAsWritten()->NumTemplateArgs)); - return true; -} - template bool RecursiveASTVisitor::TraverseObjCProtocolLoc( ObjCProtocolLoc ProtocolLoc) { @@ -2825,31 +2891,15 @@ } }) -DEF_TRAVERSE_STMT(ConceptSpecializationExpr, { - TRY_TO(TraverseConceptReference(*S)); -}) +DEF_TRAVERSE_STMT(ConceptSpecializationExpr, + { TRY_TO(TraverseConceptReferenceHelper(*S)); }) DEF_TRAVERSE_STMT(RequiresExpr, { TRY_TO(TraverseDecl(S->getBody())); for (ParmVarDecl *Parm : S->getLocalParameters()) TRY_TO(TraverseDecl(Parm)); for (concepts::Requirement *Req : S->getRequirements()) - if (auto *TypeReq = dyn_cast(Req)) { - if (!TypeReq->isSubstitutionFailure()) - TRY_TO(TraverseTypeLoc(TypeReq->getType()->getTypeLoc())); - } else if (auto *ExprReq = dyn_cast(Req)) { - if (!ExprReq->isExprSubstitutionFailure()) - TRY_TO(TraverseStmt(ExprReq->getExpr())); - auto &RetReq = ExprReq->getReturnTypeRequirement(); - if (RetReq.isTypeConstraint()) { - TRY_TO(TraverseStmt( - RetReq.getTypeConstraint()->getImmediatelyDeclaredConstraint())); - } - } else { - auto *NestedReq = cast(Req); - if (!NestedReq->isSubstitutionFailure()) - TRY_TO(TraverseStmt(NestedReq->getConstraintExpr())); - } + TRY_TO(TraverseConceptRequirement(Req)); }) // These literals (all of them) do not need any action. diff --git a/clang/lib/Index/IndexBody.cpp b/clang/lib/Index/IndexBody.cpp --- a/clang/lib/Index/IndexBody.cpp +++ b/clang/lib/Index/IndexBody.cpp @@ -483,13 +483,10 @@ return true; } - bool VisitTemplateTypeParmDecl(TemplateTypeParmDecl *D) { - // This handles references in return type requirements of RequiresExpr. - // E.g. `requires (T x) { {*x} -> ConceptRef }` - if (auto *C = D->getTypeConstraint()) - IndexCtx.handleReference(C->getNamedConcept(), C->getConceptNameLoc(), - Parent, ParentDC); - return true; + bool TraverseTypeConstraint(const TypeConstraint *C) { + IndexCtx.handleReference(C->getNamedConcept(), C->getConceptNameLoc(), + Parent, ParentDC); + return RecursiveASTVisitor::TraverseTypeConstraint(C); } }; diff --git a/clang/unittests/Tooling/RecursiveASTVisitorTests/Concept.cpp b/clang/unittests/Tooling/RecursiveASTVisitorTests/Concept.cpp --- a/clang/unittests/Tooling/RecursiveASTVisitorTests/Concept.cpp +++ b/clang/unittests/Tooling/RecursiveASTVisitorTests/Concept.cpp @@ -7,7 +7,10 @@ //===----------------------------------------------------------------------===// #include "TestVisitor.h" +#include "clang/AST/ASTConcept.h" +#include "clang/AST/DeclTemplate.h" #include "clang/AST/ExprConcepts.h" +#include "clang/AST/Type.h" using namespace clang; @@ -18,28 +21,96 @@ ++ConceptSpecializationExprsVisited; return true; } - bool TraverseConceptReference(const ConceptReference &R) { - ++ConceptReferencesTraversed; - return true; + bool TraverseTypeConstraint(const TypeConstraint *C) { + ++TypeConstraintsTraversed; + return ExpectedLocationVisitor::TraverseTypeConstraint(C); + } + bool TraverseConceptRequirement(concepts::Requirement *R) { + ++ConceptRequirementsTraversed; + return ExpectedLocationVisitor::TraverseConceptRequirement(R); } + bool shouldVisitImplicitCode() { return ShouldVisitImplicitCode; } + int ConceptSpecializationExprsVisited = 0; - int ConceptReferencesTraversed = 0; + int TypeConstraintsTraversed = 0; + int ConceptRequirementsTraversed = 0; + bool ShouldVisitImplicitCode = false; }; -TEST(RecursiveASTVisitor, ConstrainedParameter) { +TEST(RecursiveASTVisitor, Concepts) { ConceptVisitor Visitor; + Visitor.ShouldVisitImplicitCode = true; EXPECT_TRUE(Visitor.runOver("template concept Fooable = true;\n" "template void bar(T);", ConceptVisitor::Lang_CXX2a)); - // Check that we visit the "Fooable T" template parameter's TypeConstraint's - // ImmediatelyDeclaredConstraint, which is a ConceptSpecializationExpr. + // Check that we traverse the "Fooable T" template parameter's + // TypeConstraint's ImmediatelyDeclaredConstraint, which is a + // ConceptSpecializationExpr. EXPECT_EQ(1, Visitor.ConceptSpecializationExprsVisited); - // There are two ConceptReference objects in the AST: the base subobject - // of the ConceptSpecializationExpr, and the base subobject of the - // TypeConstraint itself. To avoid traversing the concept and arguments - // multiple times, we only traverse one. - EXPECT_EQ(1, Visitor.ConceptReferencesTraversed); + // Also check we traversed the TypeConstraint that produced the expr. + EXPECT_EQ(1, Visitor.TypeConstraintsTraversed); + + Visitor = {}; // Don't visit implicit code now. + EXPECT_TRUE(Visitor.runOver("template concept Fooable = true;\n" + "template void bar(T);", + ConceptVisitor::Lang_CXX2a)); + // Check that we only visit the TypeConstraint, but not the implicitly + // generated immediately declared expression. + EXPECT_EQ(0, Visitor.ConceptSpecializationExprsVisited); + EXPECT_EQ(1, Visitor.TypeConstraintsTraversed); + + Visitor = {}; + EXPECT_TRUE(Visitor.runOver("template concept A = true;\n" + "template struct vector {};\n" + "template concept B = requires(T x) {\n" + " typename vector;\n" + " {x} -> A;\n" + " requires true;\n" + "};", + ConceptVisitor::Lang_CXX2a)); + EXPECT_EQ(3, Visitor.ConceptRequirementsTraversed); +} + +struct VisitDeclOnlyOnce : ExpectedLocationVisitor { + bool VisitConceptDecl(ConceptDecl *D) { + ++ConceptDeclsVisited; + return true; + } + + bool VisitAutoType(AutoType *) { + ++AutoTypeVisited; + return true; + } + bool VisitAutoTypeLoc(AutoTypeLoc) { + ++AutoTypeLocVisited; + return true; + } + + bool TraverseVarDecl(VarDecl *V) { + // The base traversal visits only the `TypeLoc`. + // However, in the test we also validate the underlying `QualType`. + TraverseType(V->getType()); + return ExpectedLocationVisitor::TraverseVarDecl(V); + } + + bool shouldWalkTypesOfTypeLocs() { return false; } + + int ConceptDeclsVisited = 0; + int AutoTypeVisited = 0; + int AutoTypeLocVisited = 0; +}; + +TEST(RecursiveASTVisitor, ConceptDeclInAutoType) { + // Check `AutoType` and `AutoTypeLoc` do not repeatedly traverse the + // underlying concept. + VisitDeclOnlyOnce Visitor; + Visitor.runOver("template concept A = true;\n" + "A auto i = 0;\n", + VisitDeclOnlyOnce::Lang_CXX2a); + EXPECT_EQ(1, Visitor.AutoTypeVisited); + EXPECT_EQ(1, Visitor.AutoTypeLocVisited); + EXPECT_EQ(1, Visitor.ConceptDeclsVisited); } } // end anonymous namespace