https://github.com/dzbarsky created https://github.com/llvm/llvm-project/pull/202629
AddUsing, ExtractFunction, and RemoveUsingNamespace instantiate RecursiveASTVisitor only to customize a small set of callbacks. Each CRTP instantiation emits another copy of the traversal machinery in clangd. Use DynamicRecursiveASTVisitor, which clangd already links, and keep the same callbacks and explicit base traversal. Mark each callback override so signature drift remains a compile-time error. A fully stripped Release arm64 clangd decreases from 106,365,392 to 106,158,272 bytes, saving 207,120 bytes (0.195%). The downstream all-tools multicall binary decreases from 147,861,144 to 147,761,616 bytes, saving 99,528 bytes (0.067%). All 17 AddUsingTest, ExtractFunctionTest, and RemoveUsingNamespaceTest cases pass. LLVM has no dedicated benchmark for these tweaks. Five focused test runs measured 1.489 seconds baseline versus 1.475 seconds patched mean user CPU time; wall time was noisy. Work towards #202616 >From 388dd7f63afdd877ed42730bc291186aab0ae3de Mon Sep 17 00:00:00 2001 From: David Zbarsky <[email protected]> Date: Mon, 8 Jun 2026 15:00:47 -0400 Subject: [PATCH] [clangd] Share AST traversal in refactoring tweaks AddUsing, ExtractFunction, and RemoveUsingNamespace instantiate RecursiveASTVisitor only to customize a small set of callbacks. Each CRTP instantiation emits another copy of the traversal machinery in clangd. Use DynamicRecursiveASTVisitor, which clangd already links, and keep the same callbacks and explicit base traversal. Mark each callback override so signature drift remains a compile-time error. A fully stripped Release arm64 clangd decreases from 106,365,392 to 106,158,272 bytes, saving 207,120 bytes (0.195%). The downstream all-tools multicall binary decreases from 147,861,144 to 147,761,616 bytes, saving 99,528 bytes (0.067%). All 17 AddUsingTest, ExtractFunctionTest, and RemoveUsingNamespaceTest cases pass. LLVM has no dedicated benchmark for these tweaks. Five focused test runs measured 1.489 seconds baseline versus 1.475 seconds patched mean user CPU time; wall time was noisy. --- .../clangd/refactor/tweaks/AddUsing.cpp | 10 ++++---- .../refactor/tweaks/ExtractFunction.cpp | 24 +++++++++---------- .../refactor/tweaks/RemoveUsingNamespace.cpp | 6 ++--- 3 files changed, 19 insertions(+), 21 deletions(-) diff --git a/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp b/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp index 3f43362a307a9..5e6d66ee2afaf 100644 --- a/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp +++ b/clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp @@ -14,7 +14,7 @@ #include "clang/AST/Decl.h" #include "clang/AST/Expr.h" #include "clang/AST/NestedNameSpecifier.h" -#include "clang/AST/RecursiveASTVisitor.h" +#include "clang/AST/DynamicRecursiveASTVisitor.h" #include "clang/AST/Type.h" #include "clang/AST/TypeLoc.h" #include "clang/Basic/LLVM.h" @@ -79,13 +79,13 @@ std::string AddUsing::title() const { } // Locates all "using" statements relevant to SelectionDeclContext. -class UsingFinder : public RecursiveASTVisitor<UsingFinder> { +class UsingFinder : public DynamicRecursiveASTVisitor { public: UsingFinder(std::vector<const UsingDecl *> &Results, const DeclContext *SelectionDeclContext, const SourceManager &SM) : Results(Results), SelectionDeclContext(SelectionDeclContext), SM(SM) {} - bool VisitUsingDecl(UsingDecl *D) { + bool VisitUsingDecl(UsingDecl *D) override { auto Loc = D->getUsingLoc(); if (SM.getFileID(Loc) != SM.getMainFileID()) { return true; @@ -96,7 +96,7 @@ class UsingFinder : public RecursiveASTVisitor<UsingFinder> { return true; } - bool TraverseDecl(Decl *Node) { + bool TraverseDecl(Decl *Node) override { if (!Node) return true; // There is no need to go deeper into nodes that do not enclose selection, @@ -104,7 +104,7 @@ class UsingFinder : public RecursiveASTVisitor<UsingFinder> { // insertion point. if (!Node->getDeclContext() || Node->getDeclContext()->Encloses(SelectionDeclContext)) { - return RecursiveASTVisitor<UsingFinder>::TraverseDecl(Node); + return DynamicRecursiveASTVisitor::TraverseDecl(Node); } return true; } diff --git a/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp b/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp index bc9a790232507..3a5e5566c5d38 100644 --- a/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp +++ b/clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp @@ -58,7 +58,7 @@ #include "clang/AST/DeclBase.h" #include "clang/AST/ExprCXX.h" #include "clang/AST/NestedNameSpecifier.h" -#include "clang/AST/RecursiveASTVisitor.h" +#include "clang/AST/DynamicRecursiveASTVisitor.h" #include "clang/AST/Stmt.h" #include "clang/Basic/LangOptions.h" #include "clang/Basic/SourceLocation.h" @@ -576,14 +576,13 @@ CapturedZoneInfo captureZoneInfo(const ExtractionZone &ExtZone) { // We use the ASTVisitor instead of using the selection tree since we need to // find references in the PostZone as well. // FIXME: Check which statements we don't allow to extract. - class ExtractionZoneVisitor - : public clang::RecursiveASTVisitor<ExtractionZoneVisitor> { + class ExtractionZoneVisitor : public clang::DynamicRecursiveASTVisitor { public: ExtractionZoneVisitor(const ExtractionZone &ExtZone) : ExtZone(ExtZone) { TraverseDecl(const_cast<FunctionDecl *>(ExtZone.EnclosingFunction)); } - bool TraverseStmt(Stmt *S) { + bool TraverseStmt(Stmt *S) override { if (!S) return true; bool IsRootStmt = ExtZone.isRootStmt(const_cast<const Stmt *>(S)); @@ -593,7 +592,7 @@ CapturedZoneInfo captureZoneInfo(const ExtractionZone &ExtZone) { CurrentLocation = ZoneRelative::Inside; addToLoopSwitchCounters(S, 1); // Traverse using base class's TraverseStmt - RecursiveASTVisitor::TraverseStmt(S); + DynamicRecursiveASTVisitor::TraverseStmt(S); addToLoopSwitchCounters(S, -1); // We set the current location as after since next stmt will either be a // RootStmt (handled at the beginning) or after extractionZone @@ -613,12 +612,12 @@ CapturedZoneInfo captureZoneInfo(const ExtractionZone &ExtZone) { CurNumberOfSwitch += Increment; } - bool VisitDecl(Decl *D) { + bool VisitDecl(Decl *D) override { Info.createDeclInfo(D, CurrentLocation); return true; } - bool VisitDeclRefExpr(DeclRefExpr *DRE) { + bool VisitDeclRefExpr(DeclRefExpr *DRE) override { // Find the corresponding Decl and mark it's occurrence. const Decl *D = DRE->getDecl(); auto *DeclInfo = Info.getDeclInfoFor(D); @@ -630,13 +629,13 @@ CapturedZoneInfo captureZoneInfo(const ExtractionZone &ExtZone) { return true; } - bool VisitReturnStmt(ReturnStmt *Return) { + bool VisitReturnStmt(ReturnStmt *Return) override { if (CurrentLocation == ZoneRelative::Inside) Info.HasReturnStmt = true; return true; } - bool VisitBreakStmt(BreakStmt *Break) { + bool VisitBreakStmt(BreakStmt *Break) override { // Control flow is broken if break statement is selected without any // parent loop or switch statement. if (CurrentLocation == ZoneRelative::Inside && @@ -645,7 +644,7 @@ CapturedZoneInfo captureZoneInfo(const ExtractionZone &ExtZone) { return true; } - bool VisitContinueStmt(ContinueStmt *Continue) { + bool VisitContinueStmt(ContinueStmt *Continue) override { // Control flow is broken if Continue statement is selected without any // parent loop if (CurrentLocation == ZoneRelative::Inside && !CurNumberOfNestedLoops) @@ -851,10 +850,9 @@ tooling::Replacement createForwardDeclaration(const NewFunction &ExtractedFunc, // Returns true if ExtZone contains any ReturnStmts. bool hasReturnStmt(const ExtractionZone &ExtZone) { - class ReturnStmtVisitor - : public clang::RecursiveASTVisitor<ReturnStmtVisitor> { + class ReturnStmtVisitor : public clang::DynamicRecursiveASTVisitor { public: - bool VisitReturnStmt(ReturnStmt *Return) { + bool VisitReturnStmt(ReturnStmt *Return) override { Found = true; return false; // We found the answer, abort the scan. } diff --git a/clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp b/clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp index 5baa970bcb0c5..8cf8e9aa25051 100644 --- a/clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp +++ b/clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp @@ -13,7 +13,7 @@ #include "clang/AST/Decl.h" #include "clang/AST/DeclBase.h" #include "clang/AST/DeclCXX.h" -#include "clang/AST/RecursiveASTVisitor.h" +#include "clang/AST/DynamicRecursiveASTVisitor.h" #include "clang/Basic/SourceLocation.h" #include "clang/Tooling/Core/Replacement.h" #include <optional> @@ -48,14 +48,14 @@ class RemoveUsingNamespace : public Tweak { }; REGISTER_TWEAK(RemoveUsingNamespace) -class FindSameUsings : public RecursiveASTVisitor<FindSameUsings> { +class FindSameUsings : public DynamicRecursiveASTVisitor { public: FindSameUsings(const UsingDirectiveDecl &Target, std::vector<const UsingDirectiveDecl *> &Results) : TargetNS(Target.getNominatedNamespace()), TargetCtx(Target.getDeclContext()), Results(Results) {} - bool VisitUsingDirectiveDecl(UsingDirectiveDecl *D) { + bool VisitUsingDirectiveDecl(UsingDirectiveDecl *D) override { if (D->getNominatedNamespace() != TargetNS || D->getDeclContext() != TargetCtx) return true; _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
