Give the _d_this and independent-var-count caches leak-free references. NFC (#1895)

vgvassilev · web-flow · commit 3cbe47480ad0 · 2026-07-16T17:23:23.000+03:00
Two per-derivative caches were handed to several consumers as one Expr node
that must not be parented twice -- m_ThisExprDerivative (the `_d_this` adjoint)
and the independent-variable count. A consumer that forgot to copy reintroduced
the sharing findSharedNode now asserts against.

Route _d_this through a cloneThisExprDerivative() accessor that clones on read:
it is a reused compound expression, so a structural copy is the right tool. For
the count, cache the indepVarCount VarDecl (m_IndVarCountDecl) instead of a
DeclRef to it and rebuild a fresh reference on every read -- a DeclRef is
trivially rebuilt, so no node is shared and nothing is cloned, the discipline
the m_Variables FIXME asks for. Its accumulator becomes a local threaded out of
BuildVectorModeParams.
diff --git a/include/clad/Differentiator/VectorForwardModeVisitor.h b/include/clad/Differentiator/VectorForwardModeVisitor.h
@@ -17,11 +17,13 @@ class VectorForwardModeVisitor : public BaseForwardModeVisitor {
   /// m_Variables map because all other intermediate variables will have
   /// derivatives as vectors.
   std::unordered_map<const clang::ValueDecl*, clang::Expr*> m_ParamVariables;
-  /// Expression for total number of independent variables. This also includes
-  /// the size of array independent variables which will be inferred from the
-  /// size of the corresponding clad array they provide at runtime for storing
-  /// the derivatives.
-  clang::Expr* m_IndVarCountExpr;
+  /// The generated `indepVarCount` variable (total number of independent
+  /// variables). Cached as the decl so each read rebuilds a fresh DeclRef
+  /// instead of sharing one node.
+  clang::VarDecl* m_IndVarCountDecl = nullptr;
+
+  /// Build a fresh reference to the independent-variable-count variable.
+  clang::Expr* buildIndVarCountRef() { return BuildDeclRef(m_IndVarCountDecl); }
 
 public:
   VectorForwardModeVisitor(DerivativeBuilder& builder,
@@ -55,7 +57,7 @@ class VectorForwardModeVisitor : public BaseForwardModeVisitor {
   /// function parameter types and the differentiation mode are implicitly
   /// taken from the data member variables.
   llvm::SmallVector<clang::ParmVarDecl*, 8>
-  BuildVectorModeParams(DiffParams& diffParams);
+  BuildVectorModeParams(DiffParams& diffParams, clang::Expr*& indVarCountExpr);
 
   /// Get an expression used to initialize the one-hot vector for the
   /// given index and size. A one-hot vector is a vector with all elements
@@ -83,8 +85,7 @@ class VectorForwardModeVisitor : public BaseForwardModeVisitor {
   std::string GetPushForwardFunctionSuffix() override;
   DiffMode GetPushForwardMode() override;
 
-  // Function for setting the independent variables for vector mode.
-  void SetIndependentVarsExpr(clang::Expr* IndVarCountExpr);
+  void SetIndependentVarCountDecl(clang::VarDecl* VD);
 };
 } // end namespace clad
 
diff --git a/include/clad/Differentiator/VisitorBase.h b/include/clad/Differentiator/VisitorBase.h
@@ -795,6 +795,11 @@ namespace clad {
     clang::Expr* CloneNode(const clang::Expr* E);
     /// Statement overload of the structural copy above.
     clang::Stmt* CloneNode(const clang::Stmt* S);
+    /// Return a fresh clone of the cached `_d_this` adjoint reference, so each
+    /// consumer owns its copy and never parents the one cached node twice.
+    clang::Expr* cloneThisExprDerivative() {
+      return CloneNode(m_ThisExprDerivative);
+    }
     /// A deferred CloneNode(\p N): the clone is produced only if the StmtDiff
     /// representation it is stored in is actually read, so a representation no
     /// consumer needs allocates no orphaned clone. Drop-in for CloneNode(N) in
diff --git a/lib/Differentiator/JacobianModeVisitor.cpp b/lib/Differentiator/JacobianModeVisitor.cpp
@@ -66,6 +66,10 @@ DerivativeAndOverload JacobianModeVisitor::Derive() {
   // differentiation.
   size_t nonArrayIndVarCount = 0;
 
+  // Running sum of independent-variable counts, materialized into the
+  // m_IndVarCountDecl variable below.
+  Expr* indVarCountExpr = nullptr;
+
   // Set the parameters for the derivative.
   llvm::SmallVector<ParmVarDecl*, 16> params;
   llvm::SmallVector<ParmVarDecl*, 16> derivedParams;
@@ -100,11 +104,11 @@ DerivativeAndOverload JacobianModeVisitor::Derive() {
                                            /*MemberFunctionName=*/"rows", {});
       llvm::StringRef PVDName = PVD->getName();
       if (!PVDName.contains("_clad_out_")) {
-        if (!m_IndVarCountExpr)
-          m_IndVarCountExpr = getSize;
+        if (!indVarCountExpr)
+          indVarCountExpr = getSize;
         else
-          m_IndVarCountExpr =
-              BuildOp(BinaryOperatorKind::BO_Add, m_IndVarCountExpr, getSize);
+          indVarCountExpr =
+              BuildOp(BinaryOperatorKind::BO_Add, indVarCountExpr, getSize);
       }
     } else if (PVD->getType()->isReferenceType()) {
       ParmVarDecl* derivedPVD =
@@ -136,11 +140,11 @@ DerivativeAndOverload JacobianModeVisitor::Derive() {
   // of non-array parameters.
   Expr* nonArrayIndVarCountExpr = ConstantFolder::synthesizeLiteral(
       m_Context.UnsignedLongTy, m_Context, nonArrayIndVarCount);
-  if (!m_IndVarCountExpr) {
-    m_IndVarCountExpr = nonArrayIndVarCountExpr;
+  if (!indVarCountExpr) {
+    indVarCountExpr = nonArrayIndVarCountExpr;
   } else if (nonArrayIndVarCount != 0) {
-    m_IndVarCountExpr = BuildOp(BinaryOperatorKind::BO_Add, m_IndVarCountExpr,
-                                nonArrayIndVarCountExpr);
+    indVarCountExpr = BuildOp(BinaryOperatorKind::BO_Add, indVarCountExpr,
+                              nonArrayIndVarCountExpr);
   }
 
   vectorDiffFD->setParams(
@@ -149,11 +153,11 @@ DerivativeAndOverload JacobianModeVisitor::Derive() {
 
   // Instantiate a variable indepVarCount to store the total number of
   // independent variables requested.
-  // size_t indepVarCount = m_IndVarCountExpr;
-  auto* totalIndVars = BuildVarDecl(m_Context.UnsignedLongTy, "indepVarCount",
-                                    m_IndVarCountExpr);
+  // size_t indepVarCount = indVarCountExpr;
+  auto* totalIndVars =
+      BuildVarDecl(m_Context.UnsignedLongTy, "indepVarCount", indVarCountExpr);
   addToCurrentBlock(BuildDeclStmt(totalIndVars));
-  m_IndVarCountExpr = BuildDeclRef(totalIndVars);
+  m_IndVarCountDecl = totalIndVars;
 
   for (DeclStmt* decl : adjointDecls)
     addToCurrentBlock(decl);
@@ -201,8 +205,8 @@ DerivativeAndOverload JacobianModeVisitor::Derive() {
         // Create an identity matrix for the parameter,
         // with number of rows equal to the size of the array,
         // and number of columns equal to the number of independent variables
-        llvm::SmallVector<Expr*, 3> args = {
-            getSize, CloneNode(m_IndVarCountExpr), offsetExpr};
+        llvm::SmallVector<Expr*, 3> args = {getSize, buildIndVarCountRef(),
+                                            offsetExpr};
         dVectorParam = BuildIdentityMatrixExpr(dParamType, args, loc);
 
         // Update the array independent expression. getSize is already used by
@@ -215,8 +219,7 @@ DerivativeAndOverload JacobianModeVisitor::Derive() {
                       CloneNode(getSize));
       } else {
         // Create a one hot vector for the parameter.
-        llvm::SmallVector<Expr*, 2> args = {CloneNode(m_IndVarCountExpr),
-                                            offsetExpr};
+        llvm::SmallVector<Expr*, 2> args = {buildIndVarCountRef(), offsetExpr};
         dVectorParam = BuildCallExprToCladFunction("one_hot_vector", args,
                                                    {dParamType}, loc);
         ++nonArrayIndVarCount;
@@ -229,7 +232,7 @@ DerivativeAndOverload JacobianModeVisitor::Derive() {
         continue;
       // This parameter is not an independent variable.
       // Initialize by all zeros.
-      Expr* dCount = CloneNode(m_IndVarCountExpr);
+      Expr* dCount = buildIndVarCountRef();
       dVectorParam = BuildCallExprToCladFunction("zero_vector", {dCount},
                                                  {dParamType}, loc);
     }
diff --git a/lib/Differentiator/ReverseModeVisitor.cpp b/lib/Differentiator/ReverseModeVisitor.cpp
@@ -580,7 +580,7 @@ Expr* ReverseModeVisitor::getStdInitListSizeExpr(const Expr* E) {
     // ```
     if (!CI->isMemberInitializer()) {
       beginBlock(direction::reverse);
-      Expr* dthisObj = BuildOp(UO_Deref, CloneNode(m_ThisExprDerivative));
+      Expr* dthisObj = BuildOp(UO_Deref, cloneThisExprDerivative());
       StmtDiff initDiff = Visit(CI->getInit(), dthisObj);
       // Build the placement new.
       Expr* initCall = nullptr;
@@ -618,7 +618,7 @@ Expr* ReverseModeVisitor::getStdInitListSizeExpr(const Expr* E) {
     }
     llvm::StringRef fieldName = CI->getMember()->getName();
     Expr* memberDiff = utils::BuildMemberExpr(
-        m_Sema, getCurrentScope(), CloneNode(m_ThisExprDerivative), fieldName);
+        m_Sema, getCurrentScope(), cloneThisExprDerivative(), fieldName);
 
     beginBlock(direction::reverse);
     QualType memberTy = CI->getMember()->getType();
@@ -635,9 +635,8 @@ Expr* ReverseModeVisitor::getStdInitListSizeExpr(const Expr* E) {
       Expr* member = utils::BuildMemberExpr(m_Sema, getCurrentScope(), thisExpr,
                                             fieldName);
       init = BuildOp(BO_Assign, member, initDiff.getExpr());
-      Expr* memberDx =
-          utils::BuildMemberExpr(m_Sema, getCurrentScope(),
-                                 CloneNode(m_ThisExprDerivative), fieldName);
+      Expr* memberDx = utils::BuildMemberExpr(
+          m_Sema, getCurrentScope(), cloneThisExprDerivative(), fieldName);
       if (!memberDx->getType()->isRealType())
         initDx = BuildOp(BO_Assign, memberDx, initDiff.getExpr_dx());
     }
@@ -4778,7 +4777,7 @@ Expr* ReverseModeVisitor::getStdInitListSizeExpr(const Expr* E) {
     }
     // m_ThisExprDerivative is a single cached `_d_this` ref; hand out a fresh
     // clone so distinct member accesses do not share the same base node.
-    return {clonedCTE, CloneNode(m_ThisExprDerivative)};
+    return {clonedCTE, cloneThisExprDerivative()};
   }
 
   StmtDiff ReverseModeVisitor::VisitCXXTemporaryObjectExpr(
diff --git a/lib/Differentiator/VectorForwardModeVisitor.cpp b/lib/Differentiator/VectorForwardModeVisitor.cpp
@@ -16,7 +16,7 @@ using namespace clang;
 namespace clad {
 VectorForwardModeVisitor::VectorForwardModeVisitor(DerivativeBuilder& builder,
                                                    const DiffRequest& request)
-    : BaseForwardModeVisitor(builder, request), m_IndVarCountExpr(nullptr) {}
+    : BaseForwardModeVisitor(builder, request) {}
 
 VectorForwardModeVisitor::~VectorForwardModeVisitor() {}
 
@@ -28,8 +28,8 @@ DiffMode VectorForwardModeVisitor::GetPushForwardMode() {
   return DiffMode::vector_pushforward;
 }
 
-void VectorForwardModeVisitor::SetIndependentVarsExpr(Expr* IndVarCountExpr) {
-  m_IndVarCountExpr = IndVarCountExpr;
+void VectorForwardModeVisitor::SetIndependentVarCountDecl(VarDecl* VD) {
+  m_IndVarCountDecl = VD;
 }
 
 DerivativeAndOverload VectorForwardModeVisitor::Derive() {
@@ -70,7 +70,10 @@ DerivativeAndOverload VectorForwardModeVisitor::Derive() {
   DiffParams args{};
   for (const auto& dParam : m_DiffReq.DVI)
     args.push_back(dParam.param);
-  auto params = BuildVectorModeParams(args);
+  // Running sum of independent-variable counts, filled in by
+  // BuildVectorModeParams and materialized into m_IndVarCountDecl below.
+  Expr* indVarCountExpr = nullptr;
+  auto params = BuildVectorModeParams(args, indVarCountExpr);
   vectorDiffFD->setParams(
       clad_compat::makeArrayRef(params.data(), params.size()));
   vectorDiffFD->setBody(nullptr);
@@ -82,11 +85,11 @@ DerivativeAndOverload VectorForwardModeVisitor::Derive() {
 
   // Instantiate a variable indepVarCount to store the total number of
   // independent variables requested.
-  // size_t indepVarCount = m_IndVarCountExpr;
-  auto* totalIndVars = BuildVarDecl(m_Context.UnsignedLongTy, "indepVarCount",
-                                    m_IndVarCountExpr);
+  // size_t indepVarCount = indVarCountExpr;
+  auto* totalIndVars =
+      BuildVarDecl(m_Context.UnsignedLongTy, "indepVarCount", indVarCountExpr);
   addToCurrentBlock(BuildDeclStmt(totalIndVars));
-  m_IndVarCountExpr = BuildDeclRef(totalIndVars);
+  m_IndVarCountDecl = totalIndVars;
 
   // Expression for maintaining the number of independent variables processed
   // till now present as array elements. This will be sum of sizes of all such
@@ -128,8 +131,8 @@ DerivativeAndOverload VectorForwardModeVisitor::Derive() {
         // Create an identity matrix for the parameter,
         // with number of rows equal to the size of the array,
         // and number of columns equal to the number of independent variables
-        llvm::SmallVector<Expr*, 3> args = {
-            getSize, CloneNode(m_IndVarCountExpr), offsetExpr};
+        llvm::SmallVector<Expr*, 3> args = {getSize, buildIndVarCountRef(),
+                                            offsetExpr};
         dVectorParam = BuildIdentityMatrixExpr(dParamType, args, loc);
 
         // Update the array independent expression. getSize is already used by
@@ -144,8 +147,7 @@ DerivativeAndOverload VectorForwardModeVisitor::Derive() {
         }
       } else {
         // Create a one hot vector for the parameter.
-        llvm::SmallVector<Expr*, 2> args = {CloneNode(m_IndVarCountExpr),
-                                            offsetExpr};
+        llvm::SmallVector<Expr*, 2> args = {buildIndVarCountRef(), offsetExpr};
         dVectorParam = BuildCallExprToCladFunction("one_hot_vector", args,
                                                    {dParamType}, loc);
         ++nonArrayIndVarCount;
@@ -158,7 +160,7 @@ DerivativeAndOverload VectorForwardModeVisitor::Derive() {
         continue;
       // This parameter is not an independent variable.
       // Initialize by all zeros.
-      Expr* dCount = CloneNode(m_IndVarCountExpr);
+      Expr* dCount = buildIndVarCountRef();
       dVectorParam = BuildCallExprToCladFunction("zero_vector", {dCount},
                                                  {dParamType}, loc);
     }
@@ -356,7 +358,8 @@ VectorForwardModeVisitor::CreateVectorModeOverload(FunctionDecl* derivative) {
 }
 
 llvm::SmallVector<clang::ParmVarDecl*, 8>
-VectorForwardModeVisitor::BuildVectorModeParams(DiffParams& diffParams) {
+VectorForwardModeVisitor::BuildVectorModeParams(DiffParams& diffParams,
+                                                Expr*& indVarCountExpr) {
   llvm::SmallVector<clang::ParmVarDecl*, 8> params, paramDerivatives;
   params.reserve(m_DiffReq->getNumParams() + diffParams.size());
   auto derivativeFnType = cast<FunctionProtoType>(m_Derivative->getType());
@@ -395,14 +398,14 @@ VectorForwardModeVisitor::BuildVectorModeParams(DiffParams& diffParams) {
     if (utils::isArrayOrPointerType(PVD->getType())) {
       m_ParamVariables[*it] = (Expr*)BuildDeclRef(dPVD);
       // dPVD will be a clad::array or clad::array_ref, both have size() method.
-      // If m_IndVarCountExpr is null, initialize it with dPVD.size().
+      // If indVarCountExpr is null, initialize it with dPVD.size().
       // Otherwise, increment it by dPVD.size().
       Expr* getSize = BuildArrayRefSizeExpr(m_ParamVariables[*it]);
-      if (!m_IndVarCountExpr) {
-        m_IndVarCountExpr = getSize;
+      if (!indVarCountExpr) {
+        indVarCountExpr = getSize;
       } else {
-        m_IndVarCountExpr =
-            BuildOp(BinaryOperatorKind::BO_Add, m_IndVarCountExpr, getSize);
+        indVarCountExpr =
+            BuildOp(BinaryOperatorKind::BO_Add, indVarCountExpr, getSize);
       }
     } else {
       m_ParamVariables[*it] = BuildOp(UO_Deref, BuildDeclRef(dPVD), noLoc);
@@ -415,11 +418,11 @@ VectorForwardModeVisitor::BuildVectorModeParams(DiffParams& diffParams) {
   // of non-array parameters.
   Expr* nonArrayIndVarCountExpr = ConstantFolder::synthesizeLiteral(
       m_Context.UnsignedLongTy, m_Context, nonArrayIndVarCount);
-  if (!m_IndVarCountExpr) {
-    m_IndVarCountExpr = nonArrayIndVarCountExpr;
+  if (!indVarCountExpr) {
+    indVarCountExpr = nonArrayIndVarCountExpr;
   } else if (nonArrayIndVarCount != 0) {
-    m_IndVarCountExpr = BuildOp(BinaryOperatorKind::BO_Add, m_IndVarCountExpr,
-                                nonArrayIndVarCountExpr);
+    indVarCountExpr = BuildOp(BinaryOperatorKind::BO_Add, indVarCountExpr,
+                              nonArrayIndVarCountExpr);
   }
 
   // insert the derivative parameters at the end of the parameter list.
@@ -564,7 +567,7 @@ VectorForwardModeVisitor::DifferentiateVarDecl(const VarDecl* VD) {
 StmtDiff VectorForwardModeVisitor::VisitFloatingLiteral(
     const clang::FloatingLiteral* FL) {
   SourceLocation fakeLoc = utils::GetValidSLoc(m_Sema);
-  Expr* dCount = CloneNode(m_IndVarCountExpr);
+  Expr* dCount = buildIndVarCountRef();
   auto* zero_vec = BuildCallExprToCladFunction("zero_vector", {dCount},
                                                {FL->getType()}, fakeLoc);
   return StmtDiff(Clone(FL), zero_vec);
@@ -573,7 +576,7 @@ StmtDiff VectorForwardModeVisitor::VisitFloatingLiteral(
 StmtDiff
 VectorForwardModeVisitor::VisitIntegerLiteral(const clang::IntegerLiteral* IL) {
   SourceLocation fakeLoc = utils::GetValidSLoc(m_Sema);
-  Expr* dCount = CloneNode(m_IndVarCountExpr);
+  Expr* dCount = buildIndVarCountRef();
   auto* zero_vec = BuildCallExprToCladFunction("zero_vector", {dCount},
                                                {IL->getType()}, fakeLoc);
   return StmtDiff(Clone(IL), zero_vec);
diff --git a/lib/Differentiator/VectorPushForwardModeVisitor.cpp b/lib/Differentiator/VectorPushForwardModeVisitor.cpp
@@ -43,7 +43,7 @@ void VectorPushForwardModeVisitor::ExecuteInsidePushforwardFunctionBlock() {
   auto* totalIndVars =
       BuildVarDecl(m_Context.UnsignedLongTy, "indepVarCount", indVarCountExpr);
   addToCurrentBlock(BuildDeclStmt(totalIndVars));
-  SetIndependentVarsExpr(BuildDeclRef(totalIndVars));
+  SetIndependentVarCountDecl(totalIndVars);
 
   BaseForwardModeVisitor::ExecuteInsidePushforwardFunctionBlock();
 }

Original file line number	Diff line number	Diff line change
`@@ -43,7 +43,7 @@ void VectorPushForwardModeVisitor::ExecuteInsidePushforwardFunctionBlock() {`
`43`	`43`	`auto* totalIndVars =`
`44`	`44`	`BuildVarDecl(m_Context.UnsignedLongTy, "indepVarCount", indVarCountExpr);`
`45`	`45`	`addToCurrentBlock(BuildDeclStmt(totalIndVars));`
`46`		`- SetIndependentVarsExpr(BuildDeclRef(totalIndVars));`
	`46`	`+ SetIndependentVarCountDecl(totalIndVars);`
`47`	`47`
`48`	`48`	`BaseForwardModeVisitor::ExecuteInsidePushforwardFunctionBlock();`
`49`	`49`	`}`