[clang][dataflow] Change transfer function to update lattice elemen…

…t in place.

Currently, the transfer function returns a new lattice element, which forces an
unnecessary copy on processing each CFG statement.

Differential Revision: https://reviews.llvm.org/D116834

clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h

@@ -39,9 +39,8 @@ namespace dataflow {
 ///  must provide the following public members:
 ///   * `LatticeT initialElement()` - returns a lattice element that models the
 ///     initial state of a basic block;
-///   * `LatticeT transfer(const Stmt *, const LatticeT &, Environment &)` -
-///     applies the analysis transfer function for a given statement and lattice
-///     element.
+///   * `void transfer(const Stmt *, LatticeT &, Environment &)` - applies the
+///     analysis transfer function for a given statement and lattice element.
 ///
 ///  `LatticeT` is a bounded join-semilattice that is used by `Derived` and must
 ///  provide the following public members:
@@ -79,11 +78,10 @@ class DataflowAnalysis : public TypeErasedDataflowAnalysis {
     return L1 == L2;
   }
 
-  TypeErasedLattice transferTypeErased(const Stmt *Stmt,
-                                       const TypeErasedLattice &E,
-                                       Environment &Env) final {
-    const Lattice &L = llvm::any_cast<const Lattice &>(E.Value);
-    return {static_cast<Derived *>(this)->transfer(Stmt, L, Env)};
+  void transferTypeErased(const Stmt *Stmt, TypeErasedLattice &E,
+                          Environment &Env) final {
+    Lattice &L = llvm::any_cast<Lattice &>(E.Value);
+    static_cast<Derived *>(this)->transfer(Stmt, L, Env);
   }
 
 private:

clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h

@@ -64,9 +64,8 @@ class TypeErasedDataflowAnalysis {
 
   /// Applies the analysis transfer function for a given statement and
   /// type-erased lattice element.
-  virtual TypeErasedLattice transferTypeErased(const Stmt *,
-                                               const TypeErasedLattice &,
-                                               Environment &) = 0;
+  virtual void transferTypeErased(const Stmt *, TypeErasedLattice &,
+                                  Environment &) = 0;
 };
 
 /// Type-erased model of the program at a given program point.

clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp

@@ -123,7 +123,7 @@ TypeErasedDataflowAnalysisState transferBlock(
     assert(S != nullptr);
 
     transfer(*S, State.Env);
-    State.Lattice = Analysis.transferTypeErased(S, State.Lattice, State.Env);
+    Analysis.transferTypeErased(S, State.Lattice, State.Env);
 
     if (HandleTransferredStmt != nullptr)
       HandleTransferredStmt(CfgStmt.getValue(), State);

clang/unittests/Analysis/FlowSensitive/MultiVarConstantPropagationTest.cpp

@@ -132,8 +132,8 @@ class ConstantPropagationAnalysis
     return ConstantPropagationLattice::bottom();
   }
 
-  ConstantPropagationLattice
-  transfer(const Stmt *S, ConstantPropagationLattice Vars, Environment &Env) {
+  void transfer(const Stmt *S, ConstantPropagationLattice &Vars,
+                Environment &Env) {
     auto matcher =
         stmt(anyOf(declStmt(hasSingleDecl(
                        varDecl(decl().bind(kVar), hasType(isInteger()),
@@ -148,7 +148,7 @@ class ConstantPropagationAnalysis
     ASTContext &Context = getASTContext();
     auto Results = match(matcher, *S, Context);
     if (Results.empty())
-      return Vars;
+      return;
     const BoundNodes &Nodes = Results[0];
 
     const auto *Var = Nodes.getNodeAs<clang::VarDecl>(kVar);
@@ -165,29 +165,20 @@ class ConstantPropagationAnalysis
         // is (it is implementation defined), so we set it to top.
         Vars[Var] = ValueLattice::top();
       }
-      return Vars;
-    }
-
-    if (Nodes.getNodeAs<clang::Expr>(kJustAssignment)) {
+    } else if (Nodes.getNodeAs<clang::Expr>(kJustAssignment)) {
       const auto *E = Nodes.getNodeAs<clang::Expr>(kRHS);
       assert(E != nullptr);
 
       Expr::EvalResult R;
       Vars[Var] = (E->EvaluateAsInt(R, Context) && R.Val.isInt())
                       ? ValueLattice(R.Val.getInt().getExtValue())
                       : ValueLattice::top();
-      return Vars;
-    }
-
-    // Any assignment involving the expression itself resets the variable to
-    // "unknown". A more advanced analysis could try to evaluate the compound
-    // assignment. For example, `x += 0` need not invalidate `x`.
-    if (Nodes.getNodeAs<clang::Expr>(kAssignment)) {
+    } else if (Nodes.getNodeAs<clang::Expr>(kAssignment)) {
+      // Any assignment involving the expression itself resets the variable to
+      // "unknown". A more advanced analysis could try to evaluate the compound
+      // assignment. For example, `x += 0` need not invalidate `x`.
       Vars[Var] = ValueLattice::top();
-      return Vars;
     }
-
-    llvm_unreachable("expected at least one bound identifier");
   }
 };
 

clang/unittests/Analysis/FlowSensitive/NoopAnalysis.h

@@ -44,9 +44,7 @@ class NoopAnalysis : public DataflowAnalysis<NoopAnalysis, NoopLattice> {
 
   static NoopLattice initialElement() { return {}; }
 
-  NoopLattice transfer(const Stmt *S, const NoopLattice &E, Environment &Env) {
-    return {};
-  }
+  void transfer(const Stmt *S, NoopLattice &E, Environment &Env) {}
 };
 
 } // namespace dataflow

clang/unittests/Analysis/FlowSensitive/SingleVarConstantPropagationTest.cpp

@@ -121,9 +121,8 @@ class ConstantPropagationAnalysis
     return ConstantPropagationLattice::bottom();
   }
 
-  ConstantPropagationLattice transfer(const Stmt *S,
-                                      const ConstantPropagationLattice &Element,
-                                      Environment &Env) {
+  void transfer(const Stmt *S, ConstantPropagationLattice &Element,
+                Environment &Env) {
     auto matcher = stmt(
         anyOf(declStmt(hasSingleDecl(varDecl(hasType(isInteger()),
                                              hasInitializer(expr().bind(kInit)))
@@ -137,38 +136,34 @@ class ConstantPropagationAnalysis
     ASTContext &Context = getASTContext();
     auto Results = match(matcher, *S, Context);
     if (Results.empty())
-      return Element;
+      return;
     const BoundNodes &Nodes = Results[0];
 
     const auto *Var = Nodes.getNodeAs<clang::VarDecl>(kVar);
     assert(Var != nullptr);
 
     if (const auto *E = Nodes.getNodeAs<clang::Expr>(kInit)) {
       Expr::EvalResult R;
-      if (E->EvaluateAsInt(R, Context) && R.Val.isInt())
-        return ConstantPropagationLattice{
-            {{Var, R.Val.getInt().getExtValue()}}};
-      return ConstantPropagationLattice::top();
-    }
-
-    if (Nodes.getNodeAs<clang::Expr>(kJustAssignment)) {
+      Element =
+          (E->EvaluateAsInt(R, Context) && R.Val.isInt())
+              ? ConstantPropagationLattice{{{Var,
+                                             R.Val.getInt().getExtValue()}}}
+              : ConstantPropagationLattice::top();
+    } else if (Nodes.getNodeAs<clang::Expr>(kJustAssignment)) {
       const auto *RHS = Nodes.getNodeAs<clang::Expr>(kRHS);
       assert(RHS != nullptr);
 
       Expr::EvalResult R;
-      if (RHS->EvaluateAsInt(R, Context) && R.Val.isInt())
-        return ConstantPropagationLattice{
-            {{Var, R.Val.getInt().getExtValue()}}};
-      return ConstantPropagationLattice::top();
-    }
-
-    // Any assignment involving the expression itself resets the variable to
-    // "unknown". A more advanced analysis could try to evaluate the compound
-    // assignment. For example, `x += 0` need not invalidate `x`.
-    if (Nodes.getNodeAs<clang::Expr>(kAssignment))
-      return ConstantPropagationLattice::top();
-
-    llvm_unreachable("expected at least one bound identifier");
+      Element =
+          (RHS->EvaluateAsInt(R, Context) && R.Val.isInt())
+              ? ConstantPropagationLattice{{{Var,
+                                             R.Val.getInt().getExtValue()}}}
+              : ConstantPropagationLattice::top();
+    } else if (Nodes.getNodeAs<clang::Expr>(kAssignment))
+      // Any assignment involving the expression itself resets the variable to
+      // "unknown". A more advanced analysis could try to evaluate the compound
+      // assignment. For example, `x += 0` need not invalidate `x`.
+      Element = ConstantPropagationLattice::top();
   }
 };
 

clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp

@@ -113,9 +113,8 @@ class NonConvergingAnalysis
 
   static NonConvergingLattice initialElement() { return {0}; }
 
-  NonConvergingLattice transfer(const Stmt *S, const NonConvergingLattice &E,
-                                Environment &Env) {
-    return {E.State + 1};
+  void transfer(const Stmt *S, NonConvergingLattice &E, Environment &Env) {
+    ++E.State;
   }
 };
 
@@ -165,15 +164,12 @@ class FunctionCallAnalysis
 
   static FunctionCallLattice initialElement() { return {}; }
 
-  FunctionCallLattice transfer(const Stmt *S, const FunctionCallLattice &E,
-                               Environment &Env) {
-    FunctionCallLattice R = E;
+  void transfer(const Stmt *S, FunctionCallLattice &E, Environment &Env) {
     if (auto *C = dyn_cast<CallExpr>(S)) {
       if (auto *F = dyn_cast<FunctionDecl>(C->getCalleeDecl())) {
-        R.CalledFunctions.insert(F->getNameInfo().getAsString());
+        E.CalledFunctions.insert(F->getNameInfo().getAsString());
       }
     }
-    return R;
   }
 };