[clang] [clang-tools-extra] [clang][nfc] Define ConstRecursiveASTVisitor twin of RecursiveASTVisitor (PR #160065)
via cfe-commits
cfe-commits at lists.llvm.org
Mon Sep 22 03:34:57 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-tidy
Author: Arseniy Zaostrovnykh (necto)
<details>
<summary>Changes</summary>
Downstream whenever we reach out for a RecursiveASTVisitor we always
have to add a few const_casts to shoe it in. This NFC patch introduces a
const version of the same CRTP class.
To reduce code duplication, I factored out all the common logic (which is
all of it) into `RecursiveASTVisitorBase` and made `RecursiveASTVisitor`
and `ConstRecursiveASTVisitor` essentially the two instances of it, that
you should use depending on whether you want to modify AST in your
visitor.
This is very similar to the DynamicRecursiveASTVisitor structure. One
point of difference is that instead of type aliases I use inheritance to
reduce the diff of this change because templated alias is not accepted
in the implementation forwarding of the overridden member functions:
`return RecursiveASTVisitor::TraverseStmt(S);` works only if
`RecursiveASTVisitor` is defined as a derived class of
`RecursiveASTVisitorBase` and not as a parametric alias. This was not an
issue for DynamicRecursiveASTVisitor because it is not parametrised
bythe `Derived` type.
Unfortunately, I did not manager to maintain a full backwards
compatibility when it comes to the `friend` declarations, you have to
befriend the `RecursiveASTVisitorBase` and not `RecursiveASTVisitor`.
Moreover, the error message is not obvious, as it speaks of the member
function being private and does not point to the `friend` declaration.
---
Patch is 113.40 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/160065.diff
10 Files Affected:
- (modified) clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.h (+5-5)
- (modified) clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp (+1-1)
- (modified) clang-tools-extra/clangd/AST.cpp (+8-8)
- (modified) clang-tools-extra/clangd/AST.h (+2-2)
- (modified) clang/include/clang/AST/RecursiveASTVisitor.h (+528-492)
- (modified) clang/include/clang/AST/StmtOpenACC.h (-1)
- (modified) clang/lib/AST/ParentMapContext.cpp (+1-1)
- (modified) clang/lib/Index/IndexBody.cpp (+1-1)
- (modified) clang/unittests/AST/RecursiveASTVisitorTest.cpp (+129)
- (modified) clang/utils/TableGen/ClangAttrEmitter.cpp (+12-10)
``````````diff
diff --git a/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.h b/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.h
index 306eca7140d1a..0c76678c5c81a 100644
--- a/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.h
+++ b/clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.h
@@ -72,7 +72,7 @@ class StmtAncestorASTVisitor
/// Accessor for DeclParents.
const DeclParentMap &getDeclToParentStmtMap() { return DeclParents; }
- friend class clang::RecursiveASTVisitor<StmtAncestorASTVisitor>;
+ friend class clang::RecursiveASTVisitorBase<StmtAncestorASTVisitor, /*Const=*/false>;
private:
StmtParentMap StmtAncestors;
@@ -98,7 +98,7 @@ class ComponentFinderASTVisitor
/// Accessor for Components.
const ComponentVector &getComponents() { return Components; }
- friend class clang::RecursiveASTVisitor<ComponentFinderASTVisitor>;
+ friend class clang::RecursiveASTVisitorBase<ComponentFinderASTVisitor, /*Const=*/false>;
private:
ComponentVector Components;
@@ -155,7 +155,7 @@ class DependencyFinderASTVisitor
return DependsOnInsideVariable;
}
- friend class clang::RecursiveASTVisitor<DependencyFinderASTVisitor>;
+ friend class clang::RecursiveASTVisitorBase<DependencyFinderASTVisitor, /*Const=*/false>;
private:
const StmtParentMap *StmtParents;
@@ -188,7 +188,7 @@ class DeclFinderASTVisitor
return Found;
}
- friend class clang::RecursiveASTVisitor<DeclFinderASTVisitor>;
+ friend class clang::RecursiveASTVisitorBase<DeclFinderASTVisitor, /*Const=*/false>;
private:
std::string Name;
@@ -340,7 +340,7 @@ class ForLoopIndexUseVisitor
private:
/// Typedef used in CRTP functions.
using VisitorBase = RecursiveASTVisitor<ForLoopIndexUseVisitor>;
- friend class RecursiveASTVisitor<ForLoopIndexUseVisitor>;
+ friend class RecursiveASTVisitorBase<ForLoopIndexUseVisitor, /*Const=*/false>;
/// Overriden methods for RecursiveASTVisitor's traversal.
bool TraverseArraySubscriptExpr(ArraySubscriptExpr *E);
diff --git a/clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp b/clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp
index d5ccbb73735ec..f823a7019259d 100644
--- a/clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp
@@ -95,7 +95,7 @@ static bool paramReferredExactlyOnce(const CXXConstructorDecl *Ctor,
/// \see ExactlyOneUsageVisitor::hasExactlyOneUsageIn()
class ExactlyOneUsageVisitor
: public RecursiveASTVisitor<ExactlyOneUsageVisitor> {
- friend class RecursiveASTVisitor<ExactlyOneUsageVisitor>;
+ friend class RecursiveASTVisitorBase<ExactlyOneUsageVisitor, /*Const=*/false>;
public:
ExactlyOneUsageVisitor(const ParmVarDecl *ParamDecl)
diff --git a/clang-tools-extra/clangd/AST.cpp b/clang-tools-extra/clangd/AST.cpp
index 0dcff2eae05e7..ccec4478ecc04 100644
--- a/clang-tools-extra/clangd/AST.cpp
+++ b/clang-tools-extra/clangd/AST.cpp
@@ -479,7 +479,7 @@ namespace {
/// not have the deduced type set. Instead, we have to go to the appropriate
/// DeclaratorDecl/FunctionDecl and work our back to the AutoType that does have
/// a deduced type set. The AST should be improved to simplify this scenario.
-class DeducedTypeVisitor : public RecursiveASTVisitor<DeducedTypeVisitor> {
+class DeducedTypeVisitor : public ConstRecursiveASTVisitor<DeducedTypeVisitor> {
SourceLocation SearchedLocation;
const HeuristicResolver *Resolver;
@@ -493,7 +493,7 @@ class DeducedTypeVisitor : public RecursiveASTVisitor<DeducedTypeVisitor> {
//- decltype(auto) i = 1;
//- auto& i = 1;
//- auto* i = &a;
- bool VisitDeclaratorDecl(DeclaratorDecl *D) {
+ bool VisitDeclaratorDecl(const DeclaratorDecl *D) {
if (!D->getTypeSourceInfo() ||
!D->getTypeSourceInfo()->getTypeLoc().getContainedAutoTypeLoc() ||
D->getTypeSourceInfo()
@@ -522,7 +522,7 @@ class DeducedTypeVisitor : public RecursiveASTVisitor<DeducedTypeVisitor> {
//- auto foo() -> int {}
//- auto foo() -> decltype(1+1) {}
//- operator auto() const { return 10; }
- bool VisitFunctionDecl(FunctionDecl *D) {
+ bool VisitFunctionDecl(const FunctionDecl *D) {
if (!D->getTypeSourceInfo())
return true;
// Loc of auto in return type (c++14).
@@ -553,7 +553,7 @@ class DeducedTypeVisitor : public RecursiveASTVisitor<DeducedTypeVisitor> {
// Handle non-auto decltype, e.g.:
// - auto foo() -> decltype(expr) {}
// - decltype(expr);
- bool VisitDecltypeTypeLoc(DecltypeTypeLoc TL) {
+ bool VisitDecltypeTypeLoc(const DecltypeTypeLoc TL) {
if (TL.getBeginLoc() != SearchedLocation)
return true;
@@ -571,7 +571,7 @@ class DeducedTypeVisitor : public RecursiveASTVisitor<DeducedTypeVisitor> {
// Handle functions/lambdas with `auto` typed parameters.
// We deduce the type if there's exactly one instantiation visible.
- bool VisitParmVarDecl(ParmVarDecl *PVD) {
+ bool VisitParmVarDecl(const ParmVarDecl *PVD) {
if (!PVD->getType()->isDependentType())
return true;
// 'auto' here does not name an AutoType, but an implicit template param.
@@ -606,7 +606,7 @@ class DeducedTypeVisitor : public RecursiveASTVisitor<DeducedTypeVisitor> {
return true;
}
- static int paramIndex(const TemplateDecl &TD, NamedDecl &Param) {
+ static int paramIndex(const TemplateDecl &TD, const NamedDecl &Param) {
unsigned I = 0;
for (auto *ND : *TD.getTemplateParameters()) {
if (&Param == ND)
@@ -620,7 +620,7 @@ class DeducedTypeVisitor : public RecursiveASTVisitor<DeducedTypeVisitor> {
};
} // namespace
-std::optional<QualType> getDeducedType(ASTContext &ASTCtx,
+std::optional<QualType> getDeducedType(const ASTContext &ASTCtx,
const HeuristicResolver *Resolver,
SourceLocation Loc) {
if (!Loc.isValid())
@@ -659,7 +659,7 @@ static NamedDecl *getOnlyInstantiationImpl(TemplateDeclTy *TD) {
return Only;
}
-NamedDecl *getOnlyInstantiation(NamedDecl *TemplatedDecl) {
+NamedDecl *getOnlyInstantiation(const NamedDecl *TemplatedDecl) {
if (TemplateDecl *TD = TemplatedDecl->getDescribedTemplate()) {
if (auto *CTD = llvm::dyn_cast<ClassTemplateDecl>(TD))
return getOnlyInstantiationImpl(CTD);
diff --git a/clang-tools-extra/clangd/AST.h b/clang-tools-extra/clangd/AST.h
index 2b83595e5b8e9..854d227958e5b 100644
--- a/clang-tools-extra/clangd/AST.h
+++ b/clang-tools-extra/clangd/AST.h
@@ -168,7 +168,7 @@ QualType declaredType(const TypeDecl *D);
/// Retrieves the deduced type at a given location (auto, decltype).
/// It will return the underlying type.
/// If the type is an undeduced auto, returns the type itself.
-std::optional<QualType> getDeducedType(ASTContext &, const HeuristicResolver *,
+std::optional<QualType> getDeducedType(const ASTContext &, const HeuristicResolver *,
SourceLocation Loc);
// Find the abbreviated-function-template `auto` within a type, or returns null.
@@ -180,7 +180,7 @@ TemplateTypeParmTypeLoc getContainedAutoParamType(TypeLoc TL);
// If TemplatedDecl is the generic body of a template, and the template has
// exactly one visible instantiation, return the instantiated body.
-NamedDecl *getOnlyInstantiation(NamedDecl *TemplatedDecl);
+NamedDecl *getOnlyInstantiation(const NamedDecl *TemplatedDecl);
/// Return attributes attached directly to a node.
std::vector<const Attr *> getAttributes(const DynTypedNode &);
diff --git a/clang/include/clang/AST/RecursiveASTVisitor.h b/clang/include/clang/AST/RecursiveASTVisitor.h
index 1d1b7f183f75a..5aa2ebf7bc8f2 100644
--- a/clang/include/clang/AST/RecursiveASTVisitor.h
+++ b/clang/include/clang/AST/RecursiveASTVisitor.h
@@ -55,6 +55,7 @@
namespace clang {
+
// A helper macro to implement short-circuiting when recursing. It
// invokes CALL_EXPR, which must be a method call, on the derived
// object (s.t. a user of RecursiveASTVisitor can override the method
@@ -153,15 +154,29 @@ isSameMethod([[maybe_unused]] FirstMethodPtrTy FirstMethodPtr,
/// By default, this visitor preorder traverses the AST. If postorder traversal
/// is needed, the \c shouldTraversePostOrder method needs to be overridden
/// to return \c true.
-template <typename Derived> class RecursiveASTVisitor {
+/// Base template for RecursiveASTVisitor that can handle both const and non-const
+/// AST node traversal. The IsConst template parameter determines whether
+/// AST node pointers are const or non-const.
+template <typename Derived, bool IsConst>
+class RecursiveASTVisitorBase {
+protected:
+ template <typename ASTNode>
+ using MaybeConst = std::conditional_t<IsConst, const ASTNode, ASTNode>;
+
public:
+ // Type aliases that vary based on IsConst template parameter
+ using StmtPtr = MaybeConst<Stmt> *;
+ using DeclPtr = MaybeConst<Decl> *;
+ using TypePtr = MaybeConst<Type> *;
+ using ExprPtr = MaybeConst<Expr> *;
+ using AttrPtr = MaybeConst<Attr> *;
+
/// A queue used for performing data recursion over statements.
/// Parameters involving this type are used to implement data
/// recursion over Stmts and Exprs within this class, and should
/// typically not be explicitly specified by derived classes.
/// The bool bit indicates whether the statement has been traversed or not.
- typedef SmallVectorImpl<llvm::PointerIntPair<Stmt *, 1, bool>>
- DataRecursionQueue;
+ using DataRecursionQueue = SmallVectorImpl<llvm::PointerIntPair<StmtPtr, 1, bool>>;
/// Return a reference to the derived class.
Derived &getDerived() { return *static_cast<Derived *>(this); }
@@ -186,7 +201,7 @@ template <typename Derived> class RecursiveASTVisitor {
/// Recursively visits an entire AST, starting from the TranslationUnitDecl.
/// \returns false if visitation was terminated early.
- bool TraverseAST(ASTContext &AST) {
+ bool TraverseAST(MaybeConst<ASTContext> &AST) {
// Currently just an alias for TraverseDecl(TUDecl), but kept in case
// we change the implementation again.
return getDerived().TraverseDecl(AST.getTranslationUnitDecl());
@@ -197,19 +212,19 @@ template <typename Derived> class RecursiveASTVisitor {
///
/// \returns false if the visitation was terminated early, true
/// otherwise (including when the argument is nullptr).
- bool TraverseStmt(Stmt *S, DataRecursionQueue *Queue = nullptr);
+ bool TraverseStmt(StmtPtr S, DataRecursionQueue *Queue = nullptr);
/// Invoked before visiting a statement or expression via data recursion.
///
/// \returns false to skip visiting the node, true otherwise.
- bool dataTraverseStmtPre(Stmt *S) { return true; }
+ bool dataTraverseStmtPre(StmtPtr S) { return true; }
/// Invoked after visiting a statement or expression via data recursion.
/// This is not invoked if the previously invoked \c dataTraverseStmtPre
/// returned false.
///
/// \returns false if the visitation was terminated early, true otherwise.
- bool dataTraverseStmtPost(Stmt *S) { return true; }
+ bool dataTraverseStmtPost(StmtPtr S) { return true; }
/// Recursively visit a type, by dispatching to
/// Traverse*Type() based on the argument's getTypeClass() property.
@@ -230,14 +245,14 @@ template <typename Derived> class RecursiveASTVisitor {
///
/// \returns false if the visitation was terminated early, true
/// otherwise (including when the argument is a Null type location).
- bool TraverseAttr(Attr *At);
+ bool TraverseAttr(AttrPtr At);
/// Recursively visit a declaration, by dispatching to
/// Traverse*Decl() based on the argument's dynamic type.
///
/// \returns false if the visitation was terminated early, true
/// otherwise (including when the argument is NULL).
- bool TraverseDecl(Decl *D);
+ bool TraverseDecl(DeclPtr D);
/// Recursively visit a C++ nested-name-specifier.
///
@@ -294,20 +309,20 @@ template <typename Derived> class RecursiveASTVisitor {
/// be overridden for clients that need access to the name.
///
/// \returns false if the visitation was terminated early, true otherwise.
- bool TraverseConstructorInitializer(CXXCtorInitializer *Init);
+ bool TraverseConstructorInitializer(MaybeConst<CXXCtorInitializer> *Init);
/// Recursively visit a lambda capture. \c Init is the expression that
/// will be used to initialize the capture.
///
/// \returns false if the visitation was terminated early, true otherwise.
- bool TraverseLambdaCapture(LambdaExpr *LE, const LambdaCapture *C,
- Expr *Init);
+ bool TraverseLambdaCapture(MaybeConst<LambdaExpr> *LE, const LambdaCapture *C,
+ MaybeConst<Expr> *Init);
/// Recursively visit the syntactic or semantic form of an
/// initialization list.
///
/// \returns false if the visitation was terminated early, true otherwise.
- bool TraverseSynOrSemInitListExpr(InitListExpr *S,
+ bool TraverseSynOrSemInitListExpr(MaybeConst<InitListExpr> *S,
DataRecursionQueue *Queue = nullptr);
/// Recursively visit an Objective-C protocol reference with location
@@ -319,14 +334,14 @@ template <typename Derived> class RecursiveASTVisitor {
/// Recursively visit concept reference with location information.
///
/// \returns false if the visitation was terminated early, true otherwise.
- bool TraverseConceptReference(ConceptReference *CR);
+ bool TraverseConceptReference(MaybeConst<ConceptReference> *CR);
// Visit concept reference.
- bool VisitConceptReference(ConceptReference *CR) { return true; }
+ bool VisitConceptReference(MaybeConst<ConceptReference> *CR) { return true; }
// ---- Methods on Attrs ----
// Visit an attribute.
- bool VisitAttr(Attr *A) { return true; }
+ bool VisitAttr(AttrPtr A) { return true; }
// Declare Traverse* and empty Visit* for all Attr classes.
#define ATTR_VISITOR_DECLS_ONLY
@@ -335,7 +350,10 @@ template <typename Derived> class RecursiveASTVisitor {
// ---- Methods on Stmts ----
- Stmt::child_range getStmtChildren(Stmt *S) { return S->children(); }
+ using MaybeConstChildRange =
+ std::conditional_t<IsConst, Stmt::const_child_range, Stmt::child_range>;
+
+ MaybeConstChildRange getStmtChildren(StmtPtr S) { return S->children(); }
private:
// Traverse the given statement. If the most-derived traverse function takes a
@@ -345,15 +363,15 @@ template <typename Derived> class RecursiveASTVisitor {
// arm call our function rather than the derived class version.
#define TRAVERSE_STMT_BASE(NAME, CLASS, VAR, QUEUE) \
(::clang::detail::has_same_member_pointer_type< \
- decltype(&RecursiveASTVisitor::Traverse##NAME), \
+ decltype(&RecursiveASTVisitorBase::Traverse##NAME), \
decltype(&Derived::Traverse##NAME)>::value \
? static_cast<std::conditional_t< \
::clang::detail::has_same_member_pointer_type< \
- decltype(&RecursiveASTVisitor::Traverse##NAME), \
+ decltype(&RecursiveASTVisitorBase::Traverse##NAME), \
decltype(&Derived::Traverse##NAME)>::value, \
- Derived &, RecursiveASTVisitor &>>(*this) \
- .Traverse##NAME(static_cast<CLASS *>(VAR), QUEUE) \
- : getDerived().Traverse##NAME(static_cast<CLASS *>(VAR)))
+ Derived &, RecursiveASTVisitorBase &>>(*this) \
+ .Traverse##NAME(const_cast<MaybeConst<CLASS> *>(static_cast<const CLASS*>(VAR)), QUEUE) \
+ : getDerived().Traverse##NAME(const_cast<MaybeConst<CLASS> *>(static_cast<const CLASS*>(VAR))))
// Try to traverse the given statement, or enqueue it if we're performing data
// recursion in the middle of traversing another statement. Can only be called
@@ -368,20 +386,20 @@ template <typename Derived> class RecursiveASTVisitor {
// Declare Traverse*() for all concrete Stmt classes.
#define ABSTRACT_STMT(STMT)
#define STMT(CLASS, PARENT) \
- bool Traverse##CLASS(CLASS *S, DataRecursionQueue *Queue = nullptr);
+ bool Traverse##CLASS(MaybeConst<CLASS> *S, DataRecursionQueue *Queue = nullptr);
#include "clang/AST/StmtNodes.inc"
// The above header #undefs ABSTRACT_STMT and STMT upon exit.
// Define WalkUpFrom*() and empty Visit*() for all Stmt classes.
- bool WalkUpFromStmt(Stmt *S) { return getDerived().VisitStmt(S); }
- bool VisitStmt(Stmt *S) { return true; }
+ bool WalkUpFromStmt(StmtPtr S) { return getDerived().VisitStmt(S); }
+ bool VisitStmt(StmtPtr S) { return true; }
#define STMT(CLASS, PARENT) \
- bool WalkUpFrom##CLASS(CLASS *S) { \
+ bool WalkUpFrom##CLASS(MaybeConst<CLASS> *S) { \
TRY_TO(WalkUpFrom##PARENT(S)); \
TRY_TO(Visit##CLASS(S)); \
return true; \
} \
- bool Visit##CLASS(CLASS *S) { return true; }
+ bool Visit##CLASS(MaybeConst<CLASS> *S) { return true; }
#include "clang/AST/StmtNodes.inc"
// ---- Methods on Types ----
@@ -390,20 +408,20 @@ template <typename Derived> class RecursiveASTVisitor {
// Declare Traverse*() for all concrete Type classes.
#define ABSTRACT_TYPE(CLASS, BASE)
#define TYPE(CLASS, BASE) \
- bool Traverse##CLASS##Type(CLASS##Type *T, bool TraverseQualifier);
+ bool Traverse##CLASS##Type(MaybeConst<CLASS##Type> *T, bool TraverseQualifier);
#include "clang/AST/TypeNodes.inc"
// The above header #undefs ABSTRACT_TYPE and TYPE upon exit.
// Define WalkUpFrom*() and empty Visit*() for all Type classes.
- bool WalkUpFromType(Type *T) { return getDerived().VisitType(T); }
- bool VisitType(Type *T) { return true; }
+ bool WalkUpFromType(TypePtr T) { return getDerived().VisitType(T); }
+ bool VisitType(TypePtr T) { return true; }
#define TYPE(CLASS, BASE) \
- bool WalkUpFrom##CLASS##Type(CLASS##Type *T) { \
+ bool WalkUpFrom##CLASS##Type(MaybeConst<CLASS##Type> *T) { \
TRY_TO(WalkUpFrom##BASE(T)); \
TRY_TO(Visit##CLASS##Type(T)); \
return true; \
} \
- bool Visit##CLASS##Type(CLASS##Type *T) { return true; }
+ bool Visit##CLASS##Type(MaybeConst<CLASS##Type> *T) { return true; }
#include "clang/AST/TypeNodes.inc"
// ---- Methods on TypeLocs ----
@@ -445,26 +463,26 @@ template <typename Derived> class RecursiveASTVisitor {
// Declare Traverse*() for all concrete Decl classes.
#define ABSTRACT_DECL(DECL)
-#define DECL(CLASS, BASE) bool Traverse##CLASS##Decl(CLASS##Decl *D);
+#define DECL(CLASS, BASE) bool Traverse##CLASS##Decl(MaybeConst<CLASS##Decl> *D);
#include "clang/AST/DeclNodes.inc"
// The above header #undefs ABSTRACT_DECL and DECL upon exit.
// Define WalkUpFrom*() and empty Visit*() for all Decl classes.
- bool WalkUpFromDecl(Decl *D) { return getDerived().VisitDecl(D); }
- bool VisitDecl(Decl *D) { return true; }
+ bool WalkUpFromDecl(DeclPtr D) { return getDerived().VisitDecl(D); }
+ bool VisitDecl(DeclPtr D) { return true; }
#define DECL(CLASS, BASE) \
- bool WalkUpFrom##CLASS##Decl(CLASS##Decl *D) { \
+ bool WalkUpFrom##CLASS##Decl(MaybeConst<CLASS##Decl> *D) { \
TRY_TO(WalkUpFrom##BASE(D)); \
TRY_TO(Visit##CLASS##Decl(D)); \
return true; \
} \
- bool Visit##CLASS##Decl(CLASS##D...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/160065
More information about the cfe-commits
mailing list