[clang][dataflow] Replace `transfer(const Stmt *, ...)` with `transfer(const CFGElement *, ...)` in `clang/Analysis/FlowSensitive`.

[martong]

clang/unittests/Analysis/FlowSensitive/MultiVarConstantPropagationTest.cpp

138–141

This is exactly the same code that you have in SingleVarConstantPropagationTest.cpp and in the rest of the files. I think this could be hoisted into a common function declared in FlowSensitive/DataflowAnalysis.h.

By the way, wouldn't it be more convenient for the client to provide a transferStmt function if the they are interested only in Stmts? (Defining both transferStmt and transfer should be a compile time error)

[wyt]

clang/unittests/Analysis/FlowSensitive/MultiVarConstantPropagationTest.cpp

138–141

Thanks for the comment!

For now, I decided not to pull this out into a common function. This seems more like a utility function that does not quite belong DataflowAnalysis.h - it would be nice to keep the code there dedicated to analysis logic. Besides, I don't think we save much since the boilerplate is just a few lines.

Also, about having another transferStmt function, at this stage I feel that it's best if we keep things compact and have just one way of doing things - the framework is constantly evolving and having multiple implementations of the same concept can be distracting.

clang/include/clang/Analysis/FlowSensitive/NoopAnalysis.h

 //===-- NoopAnalysis.h ------------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 //  This file defines a NoopAnalysis class that just uses the builtin transfer.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_ANALYSIS_FLOWSENSITIVE_NOOPANALYSIS_H
 #define LLVM_CLANG_ANALYSIS_FLOWSENSITIVE_NOOPANALYSIS_H
 
 #include "clang/AST/ASTContext.h"
-#include "clang/AST/Stmt.h"
+#include "clang/Analysis/CFG.h"
 #include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
 #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
 #include "clang/Analysis/FlowSensitive/NoopLattice.h"
 
 namespace clang {
 namespace dataflow {
 
 class NoopAnalysis : public DataflowAnalysis<NoopAnalysis, NoopLattice> {
 public:
   /// Deprecated. Use the `DataflowAnalysisOptions` constructor instead.
   NoopAnalysis(ASTContext &Context, bool ApplyBuiltinTransfer)
       : DataflowAnalysis<NoopAnalysis, NoopLattice>(Context,
                                                     ApplyBuiltinTransfer) {}
 
   /// `ApplyBuiltinTransfer` controls whether to run the built-in transfer
   /// functions that model memory during the analysis. Their results are not
   /// used by `NoopAnalysis`, but tests that need to inspect the environment
   /// should enable them.
   NoopAnalysis(ASTContext &Context, DataflowAnalysisOptions Options)
       : DataflowAnalysis<NoopAnalysis, NoopLattice>(Context, Options) {}
 
   static NoopLattice initialElement() { return {}; }
 
-  void transfer(const Stmt *S, NoopLattice &E, Environment &Env) {}
+  void transfer(const CFGElement *E, NoopLattice &L, Environment &Env) {}
 };
 
 } // namespace dataflow
 } // namespace clang
 
 #endif // LLVM_CLANG_ANALYSIS_FLOWSENSITIVE_NOOPANALYSIS_H

clang/unittests/Analysis/FlowSensitive/ChromiumCheckModelTest.cpp

 //===- ChromiumCheckModelTest.cpp -----------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 // FIXME: Move this to clang/unittests/Analysis/FlowSensitive/Models.
 
 #include "clang/Analysis/FlowSensitive/Models/ChromiumCheckModel.h"
 #include "TestingSupport.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/CFG.h"
 #include "clang/Analysis/FlowSensitive/NoopLattice.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Testing/Support/Error.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include <string>
class ModelAdaptorAnalysis
     : public DataflowAnalysis<ModelAdaptorAnalysis<Model>, NoopLattice> {
 public:
   explicit ModelAdaptorAnalysis(ASTContext &Context)
       : DataflowAnalysis<ModelAdaptorAnalysis, NoopLattice>(
             Context, /*ApplyBuiltinTransfer=*/true) {}
 
   static NoopLattice initialElement() { return NoopLattice(); }
 
-  void transfer(const Stmt *S, NoopLattice &, Environment &Env) {
-    M.transfer(S, Env);
+  void transfer(const CFGElement *E, NoopLattice &, Environment &Env) {
+    if (auto S = E->getAs<CFGStmt>()) {
+      M.transfer(S->getStmt(), Env);
+    }
   }
 
 private:
   Model M;
 };
 
 template <typename Matcher>
 void runDataflow(llvm::StringRef Code, Matcher Match) {

clang/unittests/Analysis/FlowSensitive/MultiVarConstantPropagationTest.cpp

 
 #include "TestingSupport.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/Stmt.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/CFG.h"
 #include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
 #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
 #include "clang/Analysis/FlowSensitive/DataflowLattice.h"
 #include "clang/Analysis/FlowSensitive/MapLattice.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
public:
   explicit ConstantPropagationAnalysis(ASTContext &Context)
       : DataflowAnalysis<ConstantPropagationAnalysis,
                          ConstantPropagationLattice>(Context) {}
 
   static ConstantPropagationLattice initialElement() {
     return ConstantPropagationLattice::bottom();
   }
 
-  void transfer(const Stmt *S, ConstantPropagationLattice &Vars,
+  void transfer(const CFGElement *E, ConstantPropagationLattice &Vars,
                 Environment &Env) {
+    auto CS = E->getAs<CFGStmt>();
+    if (!CS)
+      return;
+    auto S = CS->getStmt();
     auto matcher =
         stmt(anyOf(declStmt(hasSingleDecl(
                        varDecl(decl().bind(kVar), hasType(isInteger()),
                                optionally(hasInitializer(expr().bind(kInit))))
                            .bind(kDecl))),
                    binaryOperator(hasOperatorName("="), hasLHS(refToVar()),
                                   hasRHS(expr().bind(kRHS)))
                        .bind(kJustAssignment),

clang/unittests/Analysis/FlowSensitive/SingleVarConstantPropagationTest.cpp

 
 #include "TestingSupport.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/Stmt.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/CFG.h"
 #include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
 #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
 #include "clang/Analysis/FlowSensitive/DataflowLattice.h"
-#include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Testing/ADT/StringMapEntry.h"
 #include "llvm/Testing/Support/Annotations.h"
 #include "llvm/Testing/Support/Error.h"
public:
   explicit ConstantPropagationAnalysis(ASTContext &Context)
       : DataflowAnalysis<ConstantPropagationAnalysis,
                          ConstantPropagationLattice>(Context) {}
 
   static ConstantPropagationLattice initialElement() {
     return ConstantPropagationLattice::bottom();
   }
 
-  void transfer(const Stmt *S, ConstantPropagationLattice &Element,
+  void transfer(const CFGElement *E, ConstantPropagationLattice &Element,
                 Environment &Env) {
+    auto CS = E->getAs<CFGStmt>();
+    if (!CS)
+      return;
+    auto S = CS->getStmt();
     auto matcher = stmt(
         anyOf(declStmt(hasSingleDecl(varDecl(hasType(isInteger()),
                                              hasInitializer(expr().bind(kInit)))
                                          .bind(kVar))),
               binaryOperator(hasOperatorName("="), hasLHS(refToVar()),
                              hasRHS(expr().bind(kRHS)))
                   .bind(kJustAssignment),
               binaryOperator(isAssignmentOperator(), hasLHS(refToVar()))

clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp

 public:
   explicit NonConvergingAnalysis(ASTContext &Context)
       : DataflowAnalysis<NonConvergingAnalysis, NonConvergingLattice>(
             Context,
             /*ApplyBuiltinTransfer=*/false) {}
 
   static NonConvergingLattice initialElement() { return {0}; }
 
-  void transfer(const Stmt *S, NonConvergingLattice &E, Environment &Env) {
+  void transfer(const CFGElement *, NonConvergingLattice &E, Environment &) {
     ++E.State;
   }
 };
 
 TEST(DataflowAnalysisTest, NonConvergingAnalysis) {
   std::string Code = R"(
     void target() {
       while(true) {}

 class FunctionCallAnalysis
     : public DataflowAnalysis<FunctionCallAnalysis, FunctionCallLattice> {
 public:
   explicit FunctionCallAnalysis(ASTContext &Context)
       : DataflowAnalysis<FunctionCallAnalysis, FunctionCallLattice>(Context) {}
 
   static FunctionCallLattice initialElement() { return {}; }
 
-  void transfer(const Stmt *S, FunctionCallLattice &E, Environment &Env) {
+  void transfer(const CFGElement *Elt, FunctionCallLattice &E, Environment &) {
+    auto CS = Elt->getAs<CFGStmt>();
+    if (!CS)
+      return;
+    auto S = CS->getStmt();
     if (auto *C = dyn_cast<CallExpr>(S)) {
       if (auto *F = dyn_cast<FunctionDecl>(C->getCalleeDecl())) {
         E.CalledFunctions.insert(F->getNameInfo().getAsString());
       }
     }
   }
 };
 

 class SpecialBoolAnalysis
     : public DataflowAnalysis<SpecialBoolAnalysis, NoopLattice> {
 public:
   explicit SpecialBoolAnalysis(ASTContext &Context)
       : DataflowAnalysis<SpecialBoolAnalysis, NoopLattice>(Context) {}
 
   static NoopLattice initialElement() { return {}; }
 
-  void transfer(const Stmt *S, NoopLattice &, Environment &Env) {
+  void transfer(const CFGElement *Elt, NoopLattice &, Environment &Env) {
+    auto CS = Elt->getAs<CFGStmt>();
+    if (!CS)
+      return;
+    auto S = CS->getStmt();
     auto SpecialBoolRecordDecl = recordDecl(hasName("SpecialBool"));
     auto HasSpecialBoolType = hasType(SpecialBoolRecordDecl);
 
     if (const auto *E = selectFirst<CXXConstructExpr>(
             "call", match(cxxConstructExpr(HasSpecialBoolType).bind("call"), *S,
                           getASTContext()))) {
       auto &ConstructorVal = *Env.createValue(E->getType());
       ConstructorVal.setProperty("is_set", Env.getBoolLiteralValue(false));
class OptionalIntAnalysis
     : public DataflowAnalysis<OptionalIntAnalysis, NoopLattice> {
 public:
   explicit OptionalIntAnalysis(ASTContext &Context, BoolValue &HasValueTop)
       : DataflowAnalysis<OptionalIntAnalysis, NoopLattice>(Context),
         HasValueTop(HasValueTop) {}
 
   static NoopLattice initialElement() { return {}; }
 
-  void transfer(const Stmt *S, NoopLattice &, Environment &Env) {
+  void transfer(const CFGElement *Elt, NoopLattice &, Environment &Env) {
+    auto CS = Elt->getAs<CFGStmt>();
+    if (!CS)
+      return;
+    auto S = CS->getStmt();
     auto OptionalIntRecordDecl = recordDecl(hasName("OptionalInt"));
     auto HasOptionalIntType = hasType(OptionalIntRecordDecl);
 
     if (const auto *E = selectFirst<CXXConstructExpr>(
             "call", match(cxxConstructExpr(HasOptionalIntType).bind("call"), *S,
                           getASTContext()))) {
       auto &ConstructorVal = *Env.createValue(E->getType());
       ConstructorVal.setProperty("has_value", Env.getBoolLiteralValue(false));