This revision was automatically updated to reflect the committed changes.
Closed by commit rG2b833d4086ab: [AST] Improve traversal of concepts and
concept requirements (authored by ilya-biryukov).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D124532/new/
https://reviews.llvm.org/D124532
Files:
clang-tools-extra/clangd/Selection.cpp
clang-tools-extra/clangd/unittests/XRefsTests.cpp
clang/include/clang/AST/RecursiveASTVisitor.h
clang/lib/Index/IndexBody.cpp
clang/unittests/Tooling/RecursiveASTVisitorTests/Concept.cpp
Index: clang/unittests/Tooling/RecursiveASTVisitorTests/Concept.cpp
===================================================================
--- clang/unittests/Tooling/RecursiveASTVisitorTests/Concept.cpp
+++ 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 <typename T> concept Fooable = true;\n"
"template <Fooable T> 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 <typename T> concept Fooable = true;\n"
+ "template <Fooable T> 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 <class T> concept A = true;\n"
+ "template <class T> struct vector {};\n"
+ "template <class T> concept B = requires(T x) {\n"
+ " typename vector<T*>;\n"
+ " {x} -> A;\n"
+ " requires true;\n"
+ "};",
+ ConceptVisitor::Lang_CXX2a));
+ EXPECT_EQ(3, Visitor.ConceptRequirementsTraversed);
+}
+
+struct VisitDeclOnlyOnce : ExpectedLocationVisitor<VisitDeclOnlyOnce> {
+ 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 <class T> 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
Index: clang/lib/Index/IndexBody.cpp
===================================================================
--- clang/lib/Index/IndexBody.cpp
+++ 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);
}
};
Index: clang/include/clang/AST/RecursiveASTVisitor.h
===================================================================
--- clang/include/clang/AST/RecursiveASTVisitor.h
+++ 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 <typename T>
@@ -511,6 +517,54 @@
bool PostVisitStmt(Stmt *S);
};
+template <typename Derived>
+bool RecursiveASTVisitor<Derived>::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 <typename Derived>
+bool RecursiveASTVisitor<Derived>::TraverseConceptRequirement(
+ concepts::Requirement *R) {
+ switch (R->getKind()) {
+ case concepts::Requirement::RK_Type:
+ return getDerived().TraverseConceptTypeRequirement(
+ cast<concepts::TypeRequirement>(R));
+ case concepts::Requirement::RK_Simple:
+ case concepts::Requirement::RK_Compound:
+ return getDerived().TraverseConceptExprRequirement(
+ cast<concepts::ExprRequirement>(R));
+ case concepts::Requirement::RK_Nested:
+ return getDerived().TraverseConceptNestedRequirement(
+ cast<concepts::NestedRequirement>(R));
+ }
+}
+
+template <typename Derived>
+bool RecursiveASTVisitor<Derived>::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 <typename Derived>
bool RecursiveASTVisitor<Derived>::dataTraverseNode(Stmt *S,
DataRecursionQueue *Queue) {
@@ -530,6 +584,40 @@
#undef DISPATCH_STMT
+template <typename Derived>
+bool RecursiveASTVisitor<Derived>::TraverseConceptTypeRequirement(
+ concepts::TypeRequirement *R) {
+ if (R->isSubstitutionFailure())
+ return true;
+ return getDerived().TraverseTypeLoc(R->getType()->getTypeLoc());
+}
+
+template <typename Derived>
+bool RecursiveASTVisitor<Derived>::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 <typename Derived>
+bool RecursiveASTVisitor<Derived>::TraverseConceptNestedRequirement(
+ concepts::NestedRequirement *R) {
+ if (!R->isSubstitutionFailure())
+ return getDerived().TraverseStmt(R->getConstraintExpr());
+ return true;
+}
+
template <typename Derived>
bool RecursiveASTVisitor<Derived>::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 <typename Derived>
bool RecursiveASTVisitor<Derived>::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<typename Derived>
-bool RecursiveASTVisitor<Derived>::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 <typename Derived>
bool RecursiveASTVisitor<Derived>::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<concepts::TypeRequirement>(Req)) {
- if (!TypeReq->isSubstitutionFailure())
- TRY_TO(TraverseTypeLoc(TypeReq->getType()->getTypeLoc()));
- } else if (auto *ExprReq = dyn_cast<concepts::ExprRequirement>(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<concepts::NestedRequirement>(Req);
- if (!NestedReq->isSubstitutionFailure())
- TRY_TO(TraverseStmt(NestedReq->getConstraintExpr()));
- }
+ TRY_TO(TraverseConceptRequirement(Req));
})
// These literals (all of them) do not need any action.
Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -2085,6 +2085,19 @@
checkFindRefs(Code);
}
+TEST(FindReferences, ConceptReq) {
+ constexpr llvm::StringLiteral Code = R"cpp(
+ template <class T>
+ concept $def[[IsSmal^l]] = sizeof(T) <= 8;
+
+ template <class T>
+ concept IsSmallPtr = requires(T x) {
+ { *x } -> [[IsSmal^l]];
+ };
+ )cpp";
+ checkFindRefs(Code);
+}
+
TEST(FindReferences, RequiresExprParameters) {
constexpr llvm::StringLiteral Code = R"cpp(
template <class T>
Index: clang-tools-extra/clangd/Selection.cpp
===================================================================
--- clang-tools-extra/clangd/Selection.cpp
+++ 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<SelectionVisitor>;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
