Skip to content

[NFC] [Sema] [Modules] Use DynamicRecursiveASTVisitor to reduce generted code size #151074

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

ChuanqiXu9
Copy link
Member

It is better to use DynamicRecursiveASTVisitor than RecursiveASTVisitor as it can reduce the generated size. And also avoid using a template type to present callbacks to avoid generating more code too.

@ChuanqiXu9 ChuanqiXu9 requested review from nikic and AaronBallman July 29, 2025 02:32
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:modules C++20 modules and Clang Header Modules labels Jul 29, 2025
@ChuanqiXu9
Copy link
Member Author

ChuanqiXu9 commented Jul 29, 2025

From 1b4db78 . But I didn't use function_ref as it seems not safe to store functions.

@llvmbot
Copy link
Member

llvmbot commented Jul 29, 2025

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-modules

Author: Chuanqi Xu (ChuanqiXu9)

Changes

It is better to use DynamicRecursiveASTVisitor than RecursiveASTVisitor as it can reduce the generated size. And also avoid using a template type to present callbacks to avoid generating more code too.


Full diff: https://github.com/llvm/llvm-project/pull/151074.diff

1 Files Affected:

  • (modified) clang/lib/Sema/SemaModule.cpp (+6-9)
diff --git a/clang/lib/Sema/SemaModule.cpp b/clang/lib/Sema/SemaModule.cpp
index 98ebd707aae2e..574fa64f8068b 100644
--- a/clang/lib/Sema/SemaModule.cpp
+++ b/clang/lib/Sema/SemaModule.cpp
@@ -13,12 +13,13 @@
 
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/ASTMutationListener.h"
-#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/DynamicRecursiveASTVisitor.h"
 #include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Sema/ParsedAttr.h"
 #include "clang/Sema/SemaInternal.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/STLFunctionalExtras.h"
 
 using namespace clang;
 using namespace sema;
@@ -1422,14 +1423,14 @@ bool ExposureChecker::checkExposure(const CXXRecordDecl *RD, bool Diag) {
   return IsExposure;
 }
 
-template <typename CallbackTy>
-class ReferenceTULocalChecker
-    : public clang::RecursiveASTVisitor<ReferenceTULocalChecker<CallbackTy>> {
+class ReferenceTULocalChecker : public DynamicRecursiveASTVisitor {
 public:
+  using CallbackTy = std::function<void(DeclRefExpr *, ValueDecl *)>;
+
   ReferenceTULocalChecker(ExposureChecker &C, CallbackTy &&Callback)
       : Checker(C), Callback(std::move(Callback)) {}
 
-  bool VisitDeclRefExpr(DeclRefExpr *DRE) {
+  bool VisitDeclRefExpr(DeclRefExpr *DRE) override {
     ValueDecl *Referenced = DRE->getDecl();
     if (!Referenced)
       return true;
@@ -1468,10 +1469,6 @@ class ReferenceTULocalChecker
   CallbackTy Callback;
 };
 
-template <typename CallbackTy>
-ReferenceTULocalChecker(ExposureChecker &, CallbackTy &&)
-    -> ReferenceTULocalChecker<CallbackTy>;
-
 bool ExposureChecker::checkExposure(const Stmt *S, bool Diag) {
   if (!S)
     return false;

Copy link

github-actions bot commented Jul 29, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

…ated code size

It is better to use DynamicRecursiveASTVisitor than RecursiveASTVisitor
as it can reduce the generated size. And also avoid using a template
type to present callbacks to avoid generating more code too.
@ChuanqiXu9 ChuanqiXu9 force-pushed the SemaModuleExposureCheckDyanmicASTVisitor branch from e3bb753 to 86b334a Compare July 29, 2025 03:04
@cor3ntin cor3ntin requested a review from Sirraide July 29, 2025 08:56
Copy link
Contributor

@cor3ntin cor3ntin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@shafik
Copy link
Collaborator

shafik commented Jul 29, 2025

FYI there is also this: https://llvm-compile-time-tracker.com/

@ChuanqiXu9 ChuanqiXu9 merged commit 8f09b03 into llvm:main Jul 30, 2025
7 of 9 checks passed
krishna2803 pushed a commit to krishna2803/llvm-project that referenced this pull request Aug 12, 2025
…ted code size (llvm#151074)

It is better to use DynamicRecursiveASTVisitor than RecursiveASTVisitor
as it can reduce the generated size. And also avoid using a template
type to present callbacks to avoid generating more code too.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:modules C++20 modules and Clang Header Modules clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants