[clang][nfc] Define ConstRecursiveASTVisitor twin of RecursiveASTVisitor #160065
+810
−542
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
On the example of DeducedTypeVisitor. This is just a demonstration of what I would like to do downstream (but also upstream for the other unnecessary uses of RecursiveASTVisitor).
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-tools-extra @llvm/pr-subscribers-clang Author: Arseniy Zaostrovnykh (necto) ChangesDownstream whenever we reach out for a RecursiveASTVisitor we always To reduce code duplication, I factored out all the common logic (which is This is very similar to the DynamicRecursiveASTVisitor structure. One Unfortunately, I did not manager to maintain a full backwards 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:
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]
|
|
@llvm/pr-subscribers-clangd Author: Arseniy Zaostrovnykh (necto) ChangesDownstream whenever we reach out for a RecursiveASTVisitor we always To reduce code duplication, I factored out all the common logic (which is This is very similar to the DynamicRecursiveASTVisitor structure. One Unfortunately, I did not manager to maintain a full backwards 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:
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]
|
|
@llvm/pr-subscribers-clang-tidy Author: Arseniy Zaostrovnykh (necto) ChangesDownstream whenever we reach out for a RecursiveASTVisitor we always To reduce code duplication, I factored out all the common logic (which is This is very similar to the DynamicRecursiveASTVisitor structure. One Unfortunately, I did not manager to maintain a full backwards 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:
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]
|
Have you tried using I am very much against adding this unless there is a very strong motivation for it that isn’t already covered by Even if we never instantiate this new visitor upstream, the base class means there is one extra class to instantiate, and I am worried that even just parsing a huge template like this might be an issue (last time I checked, |
I have not. I am worried of the runtime performance because of the virtual dispatch overhead, but I have not benchmarked it.
Is there a trend of avoiding
Are you speaking of the compile-time regression, or are you worried it would bloat the executable size which might slow it down at runtime? |
Yes, I have converted a lot of uses of RAV to use DRAV (see #115144) and there are some more left that I still need to migrate; I should be able to get back to that soon.
Ah, I’m specifically talking about the fact that that Clang itself becomes slower to compiler the more RAVs there are.
It doesn’t really have an impact on runtime performance in the traditional sense. When I introduced DRAV, I also tried migrating just our most commonly used visitors (the one that is called into most often I believe is E.g. here when I migrated a few dozen visitors from RAV to DRAV, we observed a performance regression of about 0.1%; however, compiling Clang itself got 2% faster and Clang’s binary size decreased by 5%. If you want more background information as to why DRAV was introduced, see #93462. The tl;dr is that even just a single instantiation of RAV takes a long time to compile and is pretty bad for binary size; this was a while ago, but from what I recall, there were several instances of someone in the middle end making some changes to e.g. the inliner, and suddenly Clang’s binary size would go up by 4%... because we ended up inlining more uses of |
I finally got around to doing this now: #160290. After this change, there should only be a few visitors left that use RAV instead of DRAV. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
RecursiveASTVisitorBaseand madeRecursiveASTVisitorand
ConstRecursiveASTVisitoressentially the two instances of it, thatyou 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 ifRecursiveASTVisitoris defined as a derived class ofRecursiveASTVisitorBaseand not as a parametric alias. This was not anissue for DynamicRecursiveASTVisitor because it is not parameterized
by the
Derivedtype.Unfortunately, I did not manager to maintain a full backwards
compatibility when it comes to the
frienddeclarations, you have tobefriend the
RecursiveASTVisitorBaseand notRecursiveASTVisitor.Moreover, the error message is not obvious, as it speaks of the member
function being private and does not point to the
frienddeclaration.