sammccall created this revision.
sammccall added reviewers: hokein, chh, srhines.
Herald added subscribers: usaxena95, kadircet, arphaman.
sammccall requested review of this revision.
Herald added projects: clang, clang-tools-extra.
Herald added a subscriber: cfe-commits.
Given `int foo, bar;`, TraverseAST reveals this tree:
TranslationUnitDecl
- foo
- bar
Before this patch, with the TraversalScope set to {foo}, TraverseAST yields:
foo
After this patch it yields:
TranslationUnitDecl
- foo
Also, TraverseDecl(TranslationUnitDecl) now respects the traversal scope.
---
The main effect of this today is that clang-tidy checks that match the
translationUnitDecl(), either in order to traverse it or check
parentage, should work.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D104071
Files:
clang-tools-extra/clangd/DumpAST.cpp
clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
clang-tools-extra/clangd/unittests/TestTU.cpp
clang/include/clang/AST/ASTContext.h
clang/include/clang/AST/RecursiveASTVisitor.h
clang/unittests/Tooling/RecursiveASTVisitorTests/TraversalScope.cpp
Index: clang/unittests/Tooling/RecursiveASTVisitorTests/TraversalScope.cpp
===================================================================
--- clang/unittests/Tooling/RecursiveASTVisitorTests/TraversalScope.cpp
+++ clang/unittests/Tooling/RecursiveASTVisitorTests/TraversalScope.cpp
@@ -16,6 +16,12 @@
public:
Visitor(ASTContext *Context) { this->Context = Context; }
+ bool VisitTranslationUnitDecl(TranslationUnitDecl *D) {
+ auto &SM = D->getParentASTContext().getSourceManager();
+ Match("TU", SM.getLocForStartOfFile(SM.getMainFileID()));
+ return true;
+ }
+
bool VisitNamedDecl(NamedDecl *D) {
if (!D->isImplicit())
Match(D->getName(), D->getLocation());
@@ -41,6 +47,7 @@
Ctx.setTraversalScope({&Bar});
Visitor V(&Ctx);
+ V.ExpectMatch("TU", 1, 1);
V.DisallowMatch("foo", 2, 8);
V.ExpectMatch("bar", 3, 10);
V.ExpectMatch("baz", 4, 12);
Index: clang/include/clang/AST/RecursiveASTVisitor.h
===================================================================
--- clang/include/clang/AST/RecursiveASTVisitor.h
+++ clang/include/clang/AST/RecursiveASTVisitor.h
@@ -192,14 +192,10 @@
/// Return whether this visitor should traverse post-order.
bool shouldTraversePostOrder() const { return false; }
- /// Recursively visits an entire AST, starting from the top-level Decls
- /// in the AST traversal scope (by default, the TranslationUnitDecl).
+ /// Recursively visits an entire AST, starting from the TranslationUnitDecl.
/// \returns false if visitation was terminated early.
bool TraverseAST(ASTContext &AST) {
- for (Decl *D : AST.getTraversalScope())
- if (!getDerived().TraverseDecl(D))
- return false;
- return true;
+ return getDerived().TraverseDecl(AST.getTranslationUnitDecl());
}
/// Recursively visit a statement or expression, by
@@ -1495,12 +1491,23 @@
TRY_TO(TraverseStmt(D->getMessage()));
})
-DEF_TRAVERSE_DECL(
- TranslationUnitDecl,
- {// Code in an unnamed namespace shows up automatically in
- // decls_begin()/decls_end(). Thus we don't need to recurse on
- // D->getAnonymousNamespace().
- })
+DEF_TRAVERSE_DECL(TranslationUnitDecl, {
+ // Code in an unnamed namespace shows up automatically in
+ // decls_begin()/decls_end(). Thus we don't need to recurse on
+ // D->getAnonymousNamespace().
+
+ auto Scope = D->getASTContext().getTraversalScope();
+ bool ScopeIsTrivial =
+ Scope.size() == 1 && isa<TranslationUnitDecl>(Scope.front());
+ // If the traversal scope is nontrivial, limit the children.
+ if (!ScopeIsTrivial) {
+ ShouldVisitChildren = false; // we'll do that now
+ for (auto *Child : Scope) {
+ if (!canIgnoreChildDeclWhileTraversingDeclContext(Child))
+ TRY_TO(TraverseDecl(Child));
+ }
+ }
+})
DEF_TRAVERSE_DECL(PragmaCommentDecl, {})
Index: clang/include/clang/AST/ASTContext.h
===================================================================
--- clang/include/clang/AST/ASTContext.h
+++ clang/include/clang/AST/ASTContext.h
@@ -638,8 +638,9 @@
// RecursiveASTVisitor::TraverseAST will only visit reachable nodes, and
// getParents() will only observe reachable parent edges.
//
- // The scope is defined by a set of "top-level" declarations.
- // Initially, it is the entire TU: {getTranslationUnitDecl()}.
+ // The scope is defined by a set of "top-level" declarations which will be
+ // visible under the TranslationUnitDecl.
+ // Initially, it is the entire TU, represented by {getTranslationUnitDecl()}.
// Changing the scope clears the parent cache, which is expensive to rebuild.
std::vector<Decl *> getTraversalScope() const { return TraversalScope; }
void setTraversalScope(const std::vector<Decl *> &);
Index: clang-tools-extra/clangd/unittests/TestTU.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/TestTU.cpp
+++ clang-tools-extra/clangd/unittests/TestTU.cpp
@@ -195,6 +195,19 @@
return *Result;
}
+// RAII scoped class to disable TraversalScope for a ParsedAST.
+class TraverseHeadersToo {
+ ASTContext &Ctx;
+ std::vector<Decl *> ScopeToRestore;
+
+public:
+ TraverseHeadersToo(ParsedAST &AST)
+ : Ctx(AST.getASTContext()), ScopeToRestore(Ctx.getTraversalScope()) {
+ Ctx.setTraversalScope({Ctx.getTranslationUnitDecl()});
+ }
+ ~TraverseHeadersToo() { Ctx.setTraversalScope(std::move(ScopeToRestore)); }
+};
+
const NamedDecl &findDecl(ParsedAST &AST, llvm::StringRef QName) {
auto &Ctx = AST.getASTContext();
auto LookupDecl = [&Ctx](const DeclContext &Scope,
@@ -217,6 +230,7 @@
const NamedDecl &findDecl(ParsedAST &AST,
std::function<bool(const NamedDecl &)> Filter) {
+ TraverseHeadersToo Too(AST);
struct Visitor : RecursiveASTVisitor<Visitor> {
decltype(Filter) F;
llvm::SmallVector<const NamedDecl *, 1> Decls;
Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -248,13 +248,23 @@
return SQUARE($macroarg[[++]]y);
return $doubled[[sizeof]](sizeof(int));
}
+
+ // misc-no-recursion uses a custom traversal from the TUDecl
+ void foo();
+ void $bar[[bar]]() {
+ foo();
+ }
+ void $foo[[foo]]() {
+ bar();
+ }
)cpp");
auto TU = TestTU::withCode(Test.code());
TU.HeaderFilename = "assert.h"; // Suppress "not found" error.
TU.ClangTidyProvider = addTidyChecks("bugprone-sizeof-expression,"
"bugprone-macro-repeated-side-effects,"
"modernize-deprecated-headers,"
- "modernize-use-trailing-return-type");
+ "modernize-use-trailing-return-type,"
+ "misc-no-recursion");
EXPECT_THAT(
*TU.build().getDiagnostics(),
UnorderedElementsAre(
@@ -283,8 +293,12 @@
DiagSource(Diag::ClangTidy),
DiagName("modernize-use-trailing-return-type"),
// Verify that we don't have "[check-name]" suffix in the message.
- WithFix(FixMessage(
- "use a trailing return type for this function")))));
+ WithFix(
+ FixMessage("use a trailing return type for this function"))),
+ Diag(Test.range("foo"),
+ "function 'foo' is within a recursive call chain"),
+ Diag(Test.range("bar"),
+ "function 'bar' is within a recursive call chain")));
}
TEST(DiagnosticsTest, ClangTidyEOF) {
Index: clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
===================================================================
--- clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
@@ -82,7 +82,8 @@
// There is no need to go deeper into nodes that do not enclose selection,
// since "using" there will not affect selection, nor would it make a good
// insertion point.
- if (Node->getDeclContext()->Encloses(SelectionDeclContext)) {
+ if (!Node->getDeclContext() ||
+ Node->getDeclContext()->Encloses(SelectionDeclContext)) {
return RecursiveASTVisitor<UsingFinder>::TraverseDecl(Node);
}
return true;
Index: clang-tools-extra/clangd/DumpAST.cpp
===================================================================
--- clang-tools-extra/clangd/DumpAST.cpp
+++ clang-tools-extra/clangd/DumpAST.cpp
@@ -337,12 +337,8 @@
// Generally, these are nodes with position information (TypeLoc, not Type).
bool TraverseDecl(Decl *D) {
- return !D || isInjectedClassName(D) || traverseNode("declaration", D, [&] {
- if (isa<TranslationUnitDecl>(D))
- Base::TraverseAST(const_cast<ASTContext &>(Ctx));
- else
- Base::TraverseDecl(D);
- });
+ return !D || isInjectedClassName(D) ||
+ traverseNode("declaration", D, [&] { Base::TraverseDecl(D); });
}
bool TraverseTypeLoc(TypeLoc TL) {
return !TL || traverseNode("type", TL, [&] { Base::TraverseTypeLoc(TL); });
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
