Skip to content

[Clang] [NFC] Introduce ConstDynamicRecursiveASTVisitor (reland) #124821

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
merged 5 commits into from
Jan 29, 2025

Conversation

Sirraide
Copy link
Member

This relands #122991.

I candidly still have no idea what went wrong last time; a bunch of bots started complaining with a relatively obscure error (see below). I’ve updated the branch and tried bulding Clang using both Clang itself and GCC and encountered no problems.

FAILED: tools/clang/lib/AST/CMakeFiles/obj.clangAST.dir/DynamicRecursiveASTVisitor.cpp.o 
ccache /usr/bin/c++ -DCLANG_EXPORTS -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GLIBCXX_ASSERTIONS -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/tools/clang/lib/AST -I/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/clang/lib/AST -I/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/clang/include -I/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/tools/clang/include -I/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/include -I/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/llvm/include -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -fno-lifetime-dse -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough -Wno-uninitialized -Wno-nonnull -Wno-class-memaccess -Wno-redundant-move -Wno-pessimizing-move -Wno-noexcept-type -Wdel
 ete-non-virtual-dtor -Wsuggest-override -Wno-comment -Wno-misleading-indentation -fdiagnostics-color -ffunction-sections -fdata-sections -fno-common -Woverloaded-virtual -fno-strict-aliasing -O3 -DNDEBUG  -fno-exceptions -funwind-tables -fno-rtti -UNDEBUG -std=c++17 -MD -MT tools/clang/lib/AST/CMakeFiles/obj.clangAST.dir/DynamicRecursiveASTVisitor.cpp.o -MF tools/clang/lib/AST/CMakeFiles/obj.clangAST.dir/DynamicRecursiveASTVisitor.cpp.o.d -o tools/clang/lib/AST/CMakeFiles/obj.clangAST.dir/DynamicRecursiveASTVisitor.cpp.o -c /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/clang/lib/AST/DynamicRecursiveASTVisitor.cpp
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/clang/lib/AST/DynamicRecursiveASTVisitor.cpp: In member function ‘virtual bool clang::DynamicRecursiveASTVisitorBase<IsConst>::TraverseAST(clang::DynamicRecursiveASTVisitorBase<IsConst>::MaybeConst<clang::ASTContext>&)’:
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/clang/lib/AST/DynamicRecursiveASTVisitor.cpp:284:51: error: expected ‘;’ before ‘::’ token
  284 |         .template RecursiveASTVisitor<Impl<Const>>::Function(                  \
      |                                                   ^~
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/clang/lib/AST/DynamicRecursiveASTVisitor.cpp:284:51: note: in definition of macro ‘FORWARD_TO_BASE’
  284 |         .template RecursiveASTVisitor<Impl<Const>>::Function(                  \
      |                                                   ^~
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/clang/lib/AST/DynamicRecursiveASTVisitor.cpp:296:17: error: ‘::TraverseAST’ has not been declared
  296 | FORWARD_TO_BASE(TraverseAST, ASTContext, &)
      |                 ^~~~~~~~~~~
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/clang/lib/AST/DynamicRecursiveASTVisitor.cpp:284:53: note: in definition of macro ‘FORWARD_TO_BASE’
  284 |         .template RecursiveASTVisitor<Impl<Const>>::Function(                  \
      |                                                     ^~~~~~~~
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/clang/lib/AST/DynamicRecursiveASTVisitor.cpp: In member function ‘virtual bool clang::DynamicRecursiveASTVisitorBase<IsConst>::TraverseAttr(clang::DynamicRecursiveASTVisitorBase<IsConst>::MaybeConst<clang::Attr>*)’:
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/clang/lib/AST/DynamicRecursiveASTVisitor.cpp:284:51: error: expected ‘;’ before ‘::’ token
  284 |         .template RecursiveASTVisitor<Impl<Const>>::Function(                  \
      |                                                   ^~
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/clang/lib/AST/DynamicRecursiveASTVisitor.cpp:284:51: note: in definition of macro ‘FORWARD_TO_BASE’
  284 |         .template RecursiveASTVisitor<Impl<Const>>::Function(                  \
      |                                                   ^~
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/clang/lib/AST/DynamicRecursiveASTVisitor.cpp:297:17: error: ‘::TraverseAttr’ has not been declared
  297 | FORWARD_TO_BASE(TraverseAttr, Attr, *)
      |                 ^~~~~~~~~~~~
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/clang/lib/AST/DynamicRecursiveASTVisitor.cpp:284:53: note: in definition of macro ‘FORWARD_TO_BASE’
  284 |         .template RecursiveASTVisitor<Impl<Const>>::Function(                  \
      |                                                     ^~~~~~~~
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/clang/lib/AST/DynamicRecursiveASTVisitor.cpp: In member function ‘virtual bool clang::DynamicRecursiveASTVisitorBase<IsConst>::TraverseConstructorInitializer(clang::DynamicRecursiveASTVisitorBase<IsConst>::MaybeConst<clang::CXXCtorInitializer>*)’:
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/clang/lib/AST/DynamicRecursiveASTVisitor.cpp:284:51: error: expected ‘;’ before ‘::’ token
  284 |         .template RecursiveASTVisitor<Impl<Const>>::Function(                  \
      |                                                   ^~
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/clang/lib/AST/DynamicRecursiveASTVisitor.cpp:284:51: note: in definition of macro ‘FORWARD_TO_BASE’
  284 |         .template RecursiveASTVisitor<Impl<Const>>::Function(                  \
      |                                                   ^~
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/clang/lib/AST/DynamicRecursiveASTVisitor.cpp:298:17: error: ‘::TraverseConstructorInitializer’ has not been declared
  298 | FORWARD_TO_BASE(TraverseConstructorInitializer, CXXCtorInitializer, *)
      |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/clang/lib/AST/DynamicRecursiveASTVisitor.cpp:284:53: note: in definition of macro ‘FORWARD_TO_BASE’

@Sirraide Sirraide added the clang:frontend Language frontend issues, e.g. anything involving "Sema" label Jan 28, 2025
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:static analyzer labels Jan 28, 2025
@llvmbot
Copy link
Member

llvmbot commented Jan 28, 2025

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-static-analyzer-1

Author: None (Sirraide)

Changes

This relands #122991.

I candidly still have no idea what went wrong last time; a bunch of bots started complaining with a relatively obscure error (see below). I’ve updated the branch and tried bulding Clang using both Clang itself and GCC and encountered no problems.

FAILED: tools/clang/lib/AST/CMakeFiles/obj.clangAST.dir/DynamicRecursiveASTVisitor.cpp.o 
ccache /usr/bin/c++ -DCLANG_EXPORTS -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GLIBCXX_ASSERTIONS -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/tools/clang/lib/AST -I/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/clang/lib/AST -I/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/clang/include -I/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/tools/clang/include -I/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.build/include -I/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/llvm/include -fPIC -fno-semantic-interposition -fvisibility-inlines-hidden -Werror=date-time -fno-lifetime-dse -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wno-missing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough -Wno-uninitialized -Wno-nonnull -Wno-class-memaccess -Wno-redundant-move -Wno-pessimizing-move -Wno-noexcept-type -Wdel
 ete-non-virtual-dtor -Wsuggest-override -Wno-comment -Wno-misleading-indentation -fdiagnostics-color -ffunction-sections -fdata-sections -fno-common -Woverloaded-virtual -fno-strict-aliasing -O3 -DNDEBUG  -fno-exceptions -funwind-tables -fno-rtti -UNDEBUG -std=c++17 -MD -MT tools/clang/lib/AST/CMakeFiles/obj.clangAST.dir/DynamicRecursiveASTVisitor.cpp.o -MF tools/clang/lib/AST/CMakeFiles/obj.clangAST.dir/DynamicRecursiveASTVisitor.cpp.o.d -o tools/clang/lib/AST/CMakeFiles/obj.clangAST.dir/DynamicRecursiveASTVisitor.cpp.o -c /home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/clang/lib/AST/DynamicRecursiveASTVisitor.cpp
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/clang/lib/AST/DynamicRecursiveASTVisitor.cpp: In member function ‘virtual bool clang::DynamicRecursiveASTVisitorBase&lt;IsConst&gt;::TraverseAST(clang::DynamicRecursiveASTVisitorBase&lt;IsConst&gt;::MaybeConst&lt;clang::ASTContext&gt;&amp;)’:
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/clang/lib/AST/DynamicRecursiveASTVisitor.cpp:284:51: error: expected ‘;’ before ‘::’ token
  284 |         .template RecursiveASTVisitor&lt;Impl&lt;Const&gt;&gt;::Function(                  \
      |                                                   ^~
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/clang/lib/AST/DynamicRecursiveASTVisitor.cpp:284:51: note: in definition of macro ‘FORWARD_TO_BASE’
  284 |         .template RecursiveASTVisitor&lt;Impl&lt;Const&gt;&gt;::Function(                  \
      |                                                   ^~
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/clang/lib/AST/DynamicRecursiveASTVisitor.cpp:296:17: error: ‘::TraverseAST’ has not been declared
  296 | FORWARD_TO_BASE(TraverseAST, ASTContext, &amp;)
      |                 ^~~~~~~~~~~
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/clang/lib/AST/DynamicRecursiveASTVisitor.cpp:284:53: note: in definition of macro ‘FORWARD_TO_BASE’
  284 |         .template RecursiveASTVisitor&lt;Impl&lt;Const&gt;&gt;::Function(                  \
      |                                                     ^~~~~~~~
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/clang/lib/AST/DynamicRecursiveASTVisitor.cpp: In member function ‘virtual bool clang::DynamicRecursiveASTVisitorBase&lt;IsConst&gt;::TraverseAttr(clang::DynamicRecursiveASTVisitorBase&lt;IsConst&gt;::MaybeConst&lt;clang::Attr&gt;*)’:
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/clang/lib/AST/DynamicRecursiveASTVisitor.cpp:284:51: error: expected ‘;’ before ‘::’ token
  284 |         .template RecursiveASTVisitor&lt;Impl&lt;Const&gt;&gt;::Function(                  \
      |                                                   ^~
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/clang/lib/AST/DynamicRecursiveASTVisitor.cpp:284:51: note: in definition of macro ‘FORWARD_TO_BASE’
  284 |         .template RecursiveASTVisitor&lt;Impl&lt;Const&gt;&gt;::Function(                  \
      |                                                   ^~
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/clang/lib/AST/DynamicRecursiveASTVisitor.cpp:297:17: error: ‘::TraverseAttr’ has not been declared
  297 | FORWARD_TO_BASE(TraverseAttr, Attr, *)
      |                 ^~~~~~~~~~~~
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/clang/lib/AST/DynamicRecursiveASTVisitor.cpp:284:53: note: in definition of macro ‘FORWARD_TO_BASE’
  284 |         .template RecursiveASTVisitor&lt;Impl&lt;Const&gt;&gt;::Function(                  \
      |                                                     ^~~~~~~~
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/clang/lib/AST/DynamicRecursiveASTVisitor.cpp: In member function ‘virtual bool clang::DynamicRecursiveASTVisitorBase&lt;IsConst&gt;::TraverseConstructorInitializer(clang::DynamicRecursiveASTVisitorBase&lt;IsConst&gt;::MaybeConst&lt;clang::CXXCtorInitializer&gt;*)’:
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/clang/lib/AST/DynamicRecursiveASTVisitor.cpp:284:51: error: expected ‘;’ before ‘::’ token
  284 |         .template RecursiveASTVisitor&lt;Impl&lt;Const&gt;&gt;::Function(                  \
      |                                                   ^~
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/clang/lib/AST/DynamicRecursiveASTVisitor.cpp:284:51: note: in definition of macro ‘FORWARD_TO_BASE’
  284 |         .template RecursiveASTVisitor&lt;Impl&lt;Const&gt;&gt;::Function(                  \
      |                                                   ^~
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/clang/lib/AST/DynamicRecursiveASTVisitor.cpp:298:17: error: ‘::TraverseConstructorInitializer’ has not been declared
  298 | FORWARD_TO_BASE(TraverseConstructorInitializer, CXXCtorInitializer, *)
      |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/ompworker/bbot/openmp-offload-amdgpu-runtime/llvm.src/clang/lib/AST/DynamicRecursiveASTVisitor.cpp:284:53: note: in definition of macro ‘FORWARD_TO_BASE’

Patch is 25.45 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/124821.diff

3 Files Affected:

  • (modified) clang/include/clang/AST/DynamicRecursiveASTVisitor.h (+71-43)
  • (modified) clang/lib/AST/DynamicRecursiveASTVisitor.cpp (+87-164)
  • (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefMemberChecker.cpp (+3-3)
diff --git a/clang/include/clang/AST/DynamicRecursiveASTVisitor.h b/clang/include/clang/AST/DynamicRecursiveASTVisitor.h
index 4382d209908292..4e0ba568263bf3 100644
--- a/clang/include/clang/AST/DynamicRecursiveASTVisitor.h
+++ b/clang/include/clang/AST/DynamicRecursiveASTVisitor.h
@@ -52,7 +52,11 @@ class ASTContext;
 /// WalkUpFromX or post-order traversal).
 ///
 /// \see RecursiveASTVisitor.
-class DynamicRecursiveASTVisitor {
+template <bool IsConst> class DynamicRecursiveASTVisitorBase {
+protected:
+  template <typename ASTNode>
+  using MaybeConst = std::conditional_t<IsConst, const ASTNode, ASTNode>;
+
 public:
   /// Whether this visitor should recurse into template instantiations.
   bool ShouldVisitTemplateInstantiations = false;
@@ -68,28 +72,29 @@ class DynamicRecursiveASTVisitor {
   bool ShouldVisitLambdaBody = true;
 
 protected:
-  DynamicRecursiveASTVisitor() = default;
-  DynamicRecursiveASTVisitor(DynamicRecursiveASTVisitor &&) = default;
-  DynamicRecursiveASTVisitor(const DynamicRecursiveASTVisitor &) = default;
-  DynamicRecursiveASTVisitor &
-  operator=(DynamicRecursiveASTVisitor &&) = default;
-  DynamicRecursiveASTVisitor &
-  operator=(const DynamicRecursiveASTVisitor &) = default;
+  DynamicRecursiveASTVisitorBase() = default;
+  DynamicRecursiveASTVisitorBase(DynamicRecursiveASTVisitorBase &&) = default;
+  DynamicRecursiveASTVisitorBase(const DynamicRecursiveASTVisitorBase &) =
+      default;
+  DynamicRecursiveASTVisitorBase &
+  operator=(DynamicRecursiveASTVisitorBase &&) = default;
+  DynamicRecursiveASTVisitorBase &
+  operator=(const DynamicRecursiveASTVisitorBase &) = default;
 
 public:
   virtual void anchor();
-  virtual ~DynamicRecursiveASTVisitor() = default;
+  virtual ~DynamicRecursiveASTVisitorBase() = default;
 
   /// Recursively visits an entire AST, starting from the TranslationUnitDecl.
   /// \returns false if visitation was terminated early.
-  virtual bool TraverseAST(ASTContext &AST);
+  virtual bool TraverseAST(MaybeConst<ASTContext> &AST);
 
   /// Recursively visit an attribute, by dispatching to
   /// Traverse*Attr() based on the argument's dynamic type.
   ///
   /// \returns false if the visitation was terminated early, true
   /// otherwise (including when the argument is a Null type location).
-  virtual bool TraverseAttr(Attr *At);
+  virtual bool TraverseAttr(MaybeConst<Attr> *At);
 
   /// Recursively visit a constructor initializer.  This
   /// automatically dispatches to another visitor for the initializer
@@ -97,7 +102,8 @@ class DynamicRecursiveASTVisitor {
   /// be overridden for clients that need access to the name.
   ///
   /// \returns false if the visitation was terminated early, true otherwise.
-  virtual bool TraverseConstructorInitializer(CXXCtorInitializer *Init);
+  virtual bool
+  TraverseConstructorInitializer(MaybeConst<CXXCtorInitializer> *Init);
 
   /// Recursively visit a base specifier. This can be overridden by a
   /// subclass.
@@ -110,7 +116,7 @@ class DynamicRecursiveASTVisitor {
   ///
   /// \returns false if the visitation was terminated early, true
   /// otherwise (including when the argument is NULL).
-  virtual bool TraverseDecl(Decl *D);
+  virtual bool TraverseDecl(MaybeConst<Decl> *D);
 
   /// Recursively visit a name with its location information.
   ///
@@ -121,13 +127,15 @@ class DynamicRecursiveASTVisitor {
   /// will be used to initialize the capture.
   ///
   /// \returns false if the visitation was terminated early, true otherwise.
-  virtual bool TraverseLambdaCapture(LambdaExpr *LE, const LambdaCapture *C,
-                                     Expr *Init);
+  virtual bool TraverseLambdaCapture(MaybeConst<LambdaExpr> *LE,
+                                     const LambdaCapture *C,
+                                     MaybeConst<Expr> *Init);
 
   /// Recursively visit a C++ nested-name-specifier.
   ///
   /// \returns false if the visitation was terminated early, true otherwise.
-  virtual bool TraverseNestedNameSpecifier(NestedNameSpecifier *NNS);
+  virtual bool
+  TraverseNestedNameSpecifier(MaybeConst<NestedNameSpecifier> *NNS);
 
   /// Recursively visit a C++ nested-name-specifier with location
   /// information.
@@ -140,7 +148,7 @@ class DynamicRecursiveASTVisitor {
   ///
   /// \returns false if the visitation was terminated early, true
   /// otherwise (including when the argument is nullptr).
-  virtual bool TraverseStmt(Stmt *S);
+  virtual bool TraverseStmt(MaybeConst<Stmt> *S);
 
   /// Recursively visit a template argument and dispatch to the
   /// appropriate method for the argument type.
@@ -190,41 +198,51 @@ class DynamicRecursiveASTVisitor {
 
   /// Traverse a concept (requirement).
   virtual bool TraverseTypeConstraint(const TypeConstraint *C);
-  virtual bool TraverseConceptRequirement(concepts::Requirement *R);
-  virtual bool TraverseConceptTypeRequirement(concepts::TypeRequirement *R);
-  virtual bool TraverseConceptExprRequirement(concepts::ExprRequirement *R);
-  virtual bool TraverseConceptNestedRequirement(concepts::NestedRequirement *R);
-  virtual bool TraverseConceptReference(ConceptReference *CR);
-  virtual bool VisitConceptReference(ConceptReference *CR) { return true; }
+  virtual bool TraverseConceptRequirement(MaybeConst<concepts::Requirement> *R);
+
+  virtual bool
+  TraverseConceptTypeRequirement(MaybeConst<concepts::TypeRequirement> *R);
+
+  virtual bool
+  TraverseConceptExprRequirement(MaybeConst<concepts::ExprRequirement> *R);
+
+  virtual bool
+  TraverseConceptNestedRequirement(MaybeConst<concepts::NestedRequirement> *R);
+
+  virtual bool TraverseConceptReference(MaybeConst<ConceptReference> *CR);
+  virtual bool VisitConceptReference(MaybeConst<ConceptReference> *CR) {
+    return true;
+  }
 
   /// Visit a node.
-  virtual bool VisitAttr(Attr *A) { return true; }
-  virtual bool VisitDecl(Decl *D) { return true; }
-  virtual bool VisitStmt(Stmt *S) { return true; }
-  virtual bool VisitType(Type *T) { return true; }
+  virtual bool VisitAttr(MaybeConst<Attr> *A) { return true; }
+  virtual bool VisitDecl(MaybeConst<Decl> *D) { return true; }
+  virtual bool VisitStmt(MaybeConst<Stmt> *S) { return true; }
+  virtual bool VisitType(MaybeConst<Type> *T) { return true; }
   virtual bool VisitTypeLoc(TypeLoc TL) { return true; }
 
   /// Walk up from a node.
-  bool WalkUpFromDecl(Decl *D) { return VisitDecl(D); }
-  bool WalkUpFromStmt(Stmt *S) { return VisitStmt(S); }
-  bool WalkUpFromType(Type *T) { return VisitType(T); }
+  bool WalkUpFromDecl(MaybeConst<Decl> *D) { return VisitDecl(D); }
+  bool WalkUpFromStmt(MaybeConst<Stmt> *S) { return VisitStmt(S); }
+  bool WalkUpFromType(MaybeConst<Type> *T) { return VisitType(T); }
   bool WalkUpFromTypeLoc(TypeLoc TL) { return VisitTypeLoc(TL); }
 
   /// Invoked before visiting a statement or expression via data recursion.
   ///
   /// \returns false to skip visiting the node, true otherwise.
-  virtual bool dataTraverseStmtPre(Stmt *S) { return true; }
+  virtual bool dataTraverseStmtPre(MaybeConst<Stmt> *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.
-  virtual bool dataTraverseStmtPost(Stmt *S) { return true; }
-  virtual bool dataTraverseNode(Stmt *S);
+  virtual bool dataTraverseStmtPost(MaybeConst<Stmt> *S) { return true; }
+  virtual bool dataTraverseNode(MaybeConst<Stmt> *S);
 
 #define DEF_TRAVERSE_TMPL_INST(kind)                                           \
-  virtual bool TraverseTemplateInstantiations(kind##TemplateDecl *D);
+  virtual bool TraverseTemplateInstantiations(                                 \
+      MaybeConst<kind##TemplateDecl> *D);
   DEF_TRAVERSE_TMPL_INST(Class)
   DEF_TRAVERSE_TMPL_INST(Var)
   DEF_TRAVERSE_TMPL_INST(Function)
@@ -232,32 +250,34 @@ class DynamicRecursiveASTVisitor {
 
   // Decls.
 #define ABSTRACT_DECL(DECL)
-#define DECL(CLASS, BASE) virtual bool Traverse##CLASS##Decl(CLASS##Decl *D);
+#define DECL(CLASS, BASE)                                                      \
+  virtual bool Traverse##CLASS##Decl(MaybeConst<CLASS##Decl> *D);
 #include "clang/AST/DeclNodes.inc"
 
 #define DECL(CLASS, BASE)                                                      \
-  bool WalkUpFrom##CLASS##Decl(CLASS##Decl *D);                                \
-  virtual bool Visit##CLASS##Decl(CLASS##Decl *D) { return true; }
+  bool WalkUpFrom##CLASS##Decl(MaybeConst<CLASS##Decl> *D);                    \
+  virtual bool Visit##CLASS##Decl(MaybeConst<CLASS##Decl> *D) { return true; }
 #include "clang/AST/DeclNodes.inc"
 
   // Stmts.
 #define ABSTRACT_STMT(STMT)
-#define STMT(CLASS, PARENT) virtual bool Traverse##CLASS(CLASS *S);
+#define STMT(CLASS, PARENT) virtual bool Traverse##CLASS(MaybeConst<CLASS> *S);
 #include "clang/AST/StmtNodes.inc"
 
 #define STMT(CLASS, PARENT)                                                    \
-  bool WalkUpFrom##CLASS(CLASS *S);                                            \
-  virtual bool Visit##CLASS(CLASS *S) { return true; }
+  bool WalkUpFrom##CLASS(MaybeConst<CLASS> *S);                                \
+  virtual bool Visit##CLASS(MaybeConst<CLASS> *S) { return true; }
 #include "clang/AST/StmtNodes.inc"
 
   // Types.
 #define ABSTRACT_TYPE(CLASS, BASE)
-#define TYPE(CLASS, BASE) virtual bool Traverse##CLASS##Type(CLASS##Type *T);
+#define TYPE(CLASS, BASE)                                                      \
+  virtual bool Traverse##CLASS##Type(MaybeConst<CLASS##Type> *T);
 #include "clang/AST/TypeNodes.inc"
 
 #define TYPE(CLASS, BASE)                                                      \
-  bool WalkUpFrom##CLASS##Type(CLASS##Type *T);                                \
-  virtual bool Visit##CLASS##Type(CLASS##Type *T) { return true; }
+  bool WalkUpFrom##CLASS##Type(MaybeConst<CLASS##Type> *T);                    \
+  virtual bool Visit##CLASS##Type(MaybeConst<CLASS##Type> *T) { return true; }
 #include "clang/AST/TypeNodes.inc"
 
   // TypeLocs.
@@ -271,6 +291,14 @@ class DynamicRecursiveASTVisitor {
   virtual bool Visit##CLASS##TypeLoc(CLASS##TypeLoc TL) { return true; }
 #include "clang/AST/TypeLocNodes.def"
 };
+
+extern template class DynamicRecursiveASTVisitorBase<false>;
+extern template class DynamicRecursiveASTVisitorBase<true>;
+
+using DynamicRecursiveASTVisitor =
+    DynamicRecursiveASTVisitorBase</*Const=*/false>;
+using ConstDynamicRecursiveASTVisitor =
+    DynamicRecursiveASTVisitorBase</*Const=*/true>;
 } // namespace clang
 
 #endif // LLVM_CLANG_AST_DYNAMIC_RECURSIVE_AST_VISITOR_H
diff --git a/clang/lib/AST/DynamicRecursiveASTVisitor.cpp b/clang/lib/AST/DynamicRecursiveASTVisitor.cpp
index 8cfabd9f3e93fe..9cc0ee60c05f60 100644
--- a/clang/lib/AST/DynamicRecursiveASTVisitor.cpp
+++ b/clang/lib/AST/DynamicRecursiveASTVisitor.cpp
@@ -89,9 +89,9 @@ using namespace clang;
 //
 //   End result: RAV::TraverseCallExpr() is executed,
 namespace {
-struct Impl : RecursiveASTVisitor<Impl> {
-  DynamicRecursiveASTVisitor &Visitor;
-  Impl(DynamicRecursiveASTVisitor &Visitor) : Visitor(Visitor) {}
+template <bool Const> struct Impl : RecursiveASTVisitor<Impl<Const>> {
+  DynamicRecursiveASTVisitorBase<Const> &Visitor;
+  Impl(DynamicRecursiveASTVisitorBase<Const> &Visitor) : Visitor(Visitor) {}
 
   bool shouldVisitTemplateInstantiations() const {
     return Visitor.ShouldVisitTemplateInstantiations;
@@ -189,8 +189,10 @@ struct Impl : RecursiveASTVisitor<Impl> {
 
   // TraverseStmt() always passes in a queue, so we have no choice but to
   // accept it as a parameter here.
-  bool dataTraverseNode(Stmt *S, DataRecursionQueue * = nullptr) {
-    // But since don't support postorder traversal, we don't need it, so
+  bool dataTraverseNode(
+      Stmt *S,
+      typename RecursiveASTVisitor<Impl>::DataRecursionQueue * = nullptr) {
+    // But since we don't support postorder traversal, we don't need it, so
     // simply discard it here. This way, derived classes don't need to worry
     // about including it as a parameter that they never use.
     return Visitor.dataTraverseNode(S);
@@ -266,187 +268,108 @@ struct Impl : RecursiveASTVisitor<Impl> {
 };
 } // namespace
 
-void DynamicRecursiveASTVisitor::anchor() {}
-
-bool DynamicRecursiveASTVisitor::TraverseAST(ASTContext &AST) {
-  return Impl(*this).RecursiveASTVisitor<Impl>::TraverseAST(AST);
-}
-
-bool DynamicRecursiveASTVisitor::TraverseAttr(Attr *At) {
-  return Impl(*this).RecursiveASTVisitor<Impl>::TraverseAttr(At);
-}
-
-bool DynamicRecursiveASTVisitor::TraverseConstructorInitializer(
-    CXXCtorInitializer *Init) {
-  return Impl(*this).RecursiveASTVisitor<Impl>::TraverseConstructorInitializer(
-      Init);
-}
-
-bool DynamicRecursiveASTVisitor::TraverseDecl(Decl *D) {
-  return Impl(*this).RecursiveASTVisitor<Impl>::TraverseDecl(D);
-}
-
-bool DynamicRecursiveASTVisitor::TraverseLambdaCapture(LambdaExpr *LE,
-                                                       const LambdaCapture *C,
-                                                       Expr *Init) {
-  return Impl(*this).RecursiveASTVisitor<Impl>::TraverseLambdaCapture(LE, C,
-                                                                      Init);
-}
-
-bool DynamicRecursiveASTVisitor::TraverseStmt(Stmt *S) {
-  return Impl(*this).RecursiveASTVisitor<Impl>::TraverseStmt(S);
-}
-
-bool DynamicRecursiveASTVisitor::TraverseTemplateArgument(
-    const TemplateArgument &Arg) {
-  return Impl(*this).RecursiveASTVisitor<Impl>::TraverseTemplateArgument(Arg);
-}
-
-bool DynamicRecursiveASTVisitor::TraverseTemplateArguments(
-    ArrayRef<TemplateArgument> Args) {
-  return Impl(*this).RecursiveASTVisitor<Impl>::TraverseTemplateArguments(Args);
-}
-
-bool DynamicRecursiveASTVisitor::TraverseTemplateArgumentLoc(
-    const TemplateArgumentLoc &ArgLoc) {
-  return Impl(*this).RecursiveASTVisitor<Impl>::TraverseTemplateArgumentLoc(
-      ArgLoc);
-}
-
-bool DynamicRecursiveASTVisitor::TraverseTemplateName(TemplateName Template) {
-  return Impl(*this).RecursiveASTVisitor<Impl>::TraverseTemplateName(Template);
-}
-
-bool DynamicRecursiveASTVisitor::TraverseType(QualType T) {
-  return Impl(*this).RecursiveASTVisitor<Impl>::TraverseType(T);
-}
-
-bool DynamicRecursiveASTVisitor::TraverseTypeLoc(TypeLoc TL) {
-  return Impl(*this).RecursiveASTVisitor<Impl>::TraverseTypeLoc(TL);
-}
-
-bool DynamicRecursiveASTVisitor::TraverseTypeConstraint(
-    const TypeConstraint *C) {
-  return Impl(*this).RecursiveASTVisitor<Impl>::TraverseTypeConstraint(C);
-}
-bool DynamicRecursiveASTVisitor::TraverseObjCProtocolLoc(
-    ObjCProtocolLoc ProtocolLoc) {
-  return Impl(*this).RecursiveASTVisitor<Impl>::TraverseObjCProtocolLoc(
-      ProtocolLoc);
-}
-
-bool DynamicRecursiveASTVisitor::TraverseConceptRequirement(
-    concepts::Requirement *R) {
-  return Impl(*this).RecursiveASTVisitor<Impl>::TraverseConceptRequirement(R);
-}
-bool DynamicRecursiveASTVisitor::TraverseConceptTypeRequirement(
-    concepts::TypeRequirement *R) {
-  return Impl(*this).RecursiveASTVisitor<Impl>::TraverseConceptTypeRequirement(
-      R);
-}
-bool DynamicRecursiveASTVisitor::TraverseConceptExprRequirement(
-    concepts::ExprRequirement *R) {
-  return Impl(*this).RecursiveASTVisitor<Impl>::TraverseConceptExprRequirement(
-      R);
-}
-bool DynamicRecursiveASTVisitor::TraverseConceptNestedRequirement(
-    concepts::NestedRequirement *R) {
-  return Impl(*this)
-      .RecursiveASTVisitor<Impl>::TraverseConceptNestedRequirement(R);
-}
-
-bool DynamicRecursiveASTVisitor::TraverseConceptReference(
-    ConceptReference *CR) {
-  return Impl(*this).RecursiveASTVisitor<Impl>::TraverseConceptReference(CR);
-}
-
-bool DynamicRecursiveASTVisitor::TraverseCXXBaseSpecifier(
-    const CXXBaseSpecifier &Base) {
-  return Impl(*this).RecursiveASTVisitor<Impl>::TraverseCXXBaseSpecifier(Base);
-}
-
-bool DynamicRecursiveASTVisitor::TraverseDeclarationNameInfo(
-    DeclarationNameInfo NameInfo) {
-  return Impl(*this).RecursiveASTVisitor<Impl>::TraverseDeclarationNameInfo(
-      NameInfo);
-}
-
-bool DynamicRecursiveASTVisitor::TraverseNestedNameSpecifier(
-    NestedNameSpecifier *NNS) {
-  return Impl(*this).RecursiveASTVisitor<Impl>::TraverseNestedNameSpecifier(
-      NNS);
-}
-
-bool DynamicRecursiveASTVisitor::TraverseNestedNameSpecifierLoc(
-    NestedNameSpecifierLoc NNS) {
-  return Impl(*this).RecursiveASTVisitor<Impl>::TraverseNestedNameSpecifierLoc(
-      NNS);
+template <bool Const> void DynamicRecursiveASTVisitorBase<Const>::anchor() {}
+
+// Helper macros to forward a call to the base implementation since that
+// ends up getting very verbose otherwise.
+
+// This calls the RecursiveASTVisitor implementation of the same function,
+// stripping any 'const' that the DRAV implementation may have added since
+// the RAV implementation largely doesn't use 'const'.
+#define FORWARD_TO_BASE(Function, Type, RefOrPointer)                          \
+  template <bool Const>                                                        \
+  bool DynamicRecursiveASTVisitorBase<Const>::Function(                        \
+      MaybeConst<Type> RefOrPointer Param) {                                   \
+    return Impl<Const>(*this)                                                  \
+        .template RecursiveASTVisitor<Impl<Const>>::Function(                  \
+            const_cast<Type RefOrPointer>(Param));                             \
+  }
+
+// Same as 'FORWARD_TO_BASE', but doesn't change the parameter type in any way.
+#define FORWARD_TO_BASE_EXACT(Function, Type)                                  \
+  template <bool Const>                                                        \
+  bool DynamicRecursiveASTVisitorBase<Const>::Function(Type Param) {           \
+    return Impl<Const>(*this)                                                  \
+        .template RecursiveASTVisitor<Impl<Const>>::Function(Param);           \
+  }
+
+FORWARD_TO_BASE(TraverseAST, ASTContext, &)
+FORWARD_TO_BASE(TraverseAttr, Attr, *)
+FORWARD_TO_BASE(TraverseConstructorInitializer, CXXCtorInitializer, *)
+FORWARD_TO_BASE(TraverseDecl, Decl, *)
+FORWARD_TO_BASE(TraverseStmt, Stmt, *)
+FORWARD_TO_BASE(TraverseNestedNameSpecifier, NestedNameSpecifier, *)
+FORWARD_TO_BASE(TraverseTemplateInstantiations, ClassTemplateDecl, *)
+FORWARD_TO_BASE(TraverseTemplateInstantiations, VarTemplateDecl, *)
+FORWARD_TO_BASE(TraverseTemplateInstantiations, FunctionTemplateDecl, *)
+FORWARD_TO_BASE(TraverseConceptRequirement, concepts::Requirement, *)
+FORWARD_TO_BASE(TraverseConceptTypeRequirement, concepts::TypeRequirement, *)
+FORWARD_TO_BASE(TraverseConceptExprRequirement, concepts::ExprRequirement, *)
+FORWARD_TO_BASE(TraverseConceptReference, ConceptReference, *)
+FORWARD_TO_BASE(TraverseConceptNestedRequirement,
+                concepts::NestedRequirement, *)
+
+FORWARD_TO_BASE_EXACT(TraverseCXXBaseSpecifier, const CXXBaseSpecifier &)
+FORWARD_TO_BASE_EXACT(TraverseDeclarationNameInfo, DeclarationNameInfo)
+FORWARD_TO_BASE_EXACT(TraverseTemplateArgument, const TemplateArgument &)
+FORWARD_TO_BASE_EXACT(TraverseTemplateArguments, ArrayRef<TemplateArgument>)
+FORWARD_TO_BASE_EXACT(TraverseTemplateArgumentLoc, const TemplateArgumentLoc &)
+FORWARD_TO_BASE_EXACT(TraverseTemplateName, TemplateName)
+FORWARD_TO_BASE_EXACT(TraverseType, QualType)
+FORWARD_TO_BASE_EXACT(TraverseTypeLoc, TypeLoc)
+FORWARD_TO_BASE_EXACT(TraverseTypeConstraint, const TypeConstraint *)
+FORWARD_TO_BASE_EXACT(TraverseObjCProtocolLoc, ObjCProtocolLoc)
+FORWARD_TO_BASE_EXACT(TraverseNestedNameSpecifierLoc, NestedNameSpecifierLoc)
+
+template <bool Const>
+bool DynamicRecursiveASTVisitorBase<Const>::TraverseLambdaCapture(
+    MaybeConst<LambdaExpr> *LE, const LambdaCapture *C,
+    MaybeConst<Expr> *Init) {
+  return Impl<Const>(*this)
+      .template RecursiveASTVisitor<Impl<Const>>::TraverseLambdaCapture(
+          const_cast<LambdaExpr *>(LE), C, const_cast<Expr *>(Init));
 }
 
-bool DynamicRecursiveASTVisitor::dataTraverseNode(Stmt *S) {
-  return Impl(*this).RecursiveASTVisitor<Impl>::dataTraverseNode(S, nullptr);
+template <bool Const>
+bool DynamicRecursiveASTVisitorBase<Const>::dataTraverseNode(
+    MaybeConst<Stmt> *S) {
+  return Impl<Const>(*this)
+      .template RecursiveASTVisitor<Impl<Const>>::dataTraverseNode(
+          const_cast<S...
[truncated]

@cor3ntin
Copy link
Contributor

I don't think .template is incorrect. Maybe you hit #96364 or somethink @sdkrystian

@Sirraide
Copy link
Member Author

I don't think .template is incorrect. Maybe you hit #96364 or somethink @sdkrystian

Yeah, at first I thought it was required, apparently it’s not. GCC and Clang accept both, but I also don’t think it’s incorrect, so... I’m still somewhat confused about this

@Sirraide
Copy link
Member Author

Also, CI is currently failing on some codegen tests on windows, but that seems unrelated

@AaronBallman
Copy link
Collaborator

I don't think .template is incorrect. Maybe you hit #96364 or somethink @sdkrystian

Yeah, at first I thought it was required, apparently it’s not. GCC and Clang accept both, but I also don’t think it’s incorrect, so... I’m still somewhat confused about this

It's what I thought at first as well, but because the post-commit bots were falling over on parsing, it seemed like it was the most likely suspect for what's going wrong.

I really hope we're not in a situation where we need the template for some bots and can't have it for other bots.

Btw, precommit CI failures on Windows look unrelated to this PR.

@Sirraide
Copy link
Member Author

I really hope we're not in a situation where we need the template for some bots and can't have it for other bots.

Yeah, that’s what I’m a bit worried about too...

@cor3ntin
Copy link
Contributor

I really hope we're not in a situation where we need the template for some bots and can't have it for other bots.

Yeah, that’s what I’m a bit worried about too...

Well, at worst

using Base = RecursiveASTVisitor<Impl<Const>>; // and you can put that at class level
return Base::Function(...);

@Sirraide
Copy link
Member Author

Well, at worst

using Base = RecursiveASTVisitor<Impl<Const>>; // and you can put that at class level
return Base::Function(...);

It’d have to be Impl<Const>::Base::Function(...) because RAV is only a base class of Impl not of DRAV; that would still have a dependent NNS, wouldn’t it?

@cor3ntin
Copy link
Contributor

It’d have to be Impl<Const>::Base::Function(...) because RAV is only a base class of Impl not of DRAV; that would still have a dependent NNS, wouldn’t it?

In the same function then, a statement above

@Sirraide
Copy link
Member Author

In the same function then, a statement above

That would be an option yeah; I’m hoping it’ll accept it without the template, if not, er, then I’ll try that because I’m out of ideas otherwise.

@Sirraide
Copy link
Member Author

Alright, let’s see if the builders are fine with it now.

@Sirraide Sirraide merged commit 39a72be into llvm:main Jan 29, 2025
6 of 8 checks passed
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:static analyzer clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants