[clang] d0be47c - [clang][dataflow] Make `runDataflowReturnError()` a non-template function.

Martin Braenne via cfe-commits cfe-commits at lists.llvm.org
Tue Jul 4 05:44:57 PDT 2023


Author: Martin Braenne
Date: 2023-07-04T12:44:49Z
New Revision: d0be47c51cfdb8b94eb20279c02e8e2875380919

URL: https://github.com/llvm/llvm-project/commit/d0be47c51cfdb8b94eb20279c02e8e2875380919
DIFF: https://github.com/llvm/llvm-project/commit/d0be47c51cfdb8b94eb20279c02e8e2875380919.diff

LOG: [clang][dataflow] Make `runDataflowReturnError()` a non-template function.

It turns out this didn't need to be a template at all.

Likewise, change callers to they're non-template functions.

Also, correct / clarify some comments in RecordOps.h.

This is in response to post-commit comments on https://reviews.llvm.org/D153006.

Reviewed By: gribozavr2

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

Added: 
    

Modified: 
    clang/include/clang/Analysis/FlowSensitive/RecordOps.h
    clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp
    clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp
    clang/unittests/Analysis/FlowSensitive/TestingSupport.h
    clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Analysis/FlowSensitive/RecordOps.h b/clang/include/clang/Analysis/FlowSensitive/RecordOps.h
index f5a0a5a501c119..c9c302b9199bf2 100644
--- a/clang/include/clang/Analysis/FlowSensitive/RecordOps.h
+++ b/clang/include/clang/Analysis/FlowSensitive/RecordOps.h
@@ -23,7 +23,7 @@ namespace dataflow {
 ///
 /// This performs a deep copy, i.e. it copies every field and recurses on
 /// fields of record type. It also copies properties from the `StructValue`
-/// associated with `Dst` to the `StructValue` associated with `Src` (if these
+/// associated with `Src` to the `StructValue` associated with `Dst` (if these
 /// `StructValue`s exist).
 ///
 /// If there is a `StructValue` associated with `Dst` in the environment, this
@@ -52,6 +52,11 @@ void copyRecord(AggregateStorageLocation &Src, AggregateStorageLocation &Dst,
 /// refer to the same storage location. If `StructValue`s are associated with
 /// `Loc1` and `Loc2`, it also compares the properties on those `StructValue`s.
 ///
+/// Note on how to interpret the result:
+/// - If this returns true, the records are guaranteed to be equal at runtime.
+/// - If this returns false, the records may still be equal at runtime; our
+///   analysis merely cannot guarantee that they will be equal.
+///
 /// Requirements:
 ///
 ///  `Src` and `Dst` must have the same canonical unqualified type.

diff  --git a/clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp b/clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp
index a89011e423cd27..1f5fce1f2dd467 100644
--- a/clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp
@@ -16,14 +16,18 @@ namespace dataflow {
 namespace test {
 namespace {
 
-template <typename VerifyResultsT>
-void runDataflow(llvm::StringRef Code, VerifyResultsT VerifyResults,
-                 LangStandard::Kind Std = LangStandard::lang_cxx17,
-                 llvm::StringRef TargetFun = "target") {
+void runDataflow(
+    llvm::StringRef Code,
+    std::function<
+        void(const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &,
+             ASTContext &)>
+        VerifyResults,
+    LangStandard::Kind Std = LangStandard::lang_cxx17,
+    llvm::StringRef TargetFun = "target") {
   ASSERT_THAT_ERROR(
-      runDataflowReturnError(Code, VerifyResults,
-                             DataflowAnalysisOptions{BuiltinOptions{}}, Std,
-                             TargetFun),
+      checkDataflowWithNoopAnalysis(Code, VerifyResults,
+                                    DataflowAnalysisOptions{BuiltinOptions{}},
+                                    Std, TargetFun),
       llvm::Succeeded());
 }
 

diff  --git a/clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp b/clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp
index a88b8d88c74c07..72bdfee26fe7f3 100644
--- a/clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp
@@ -153,6 +153,43 @@ test::buildStatementToAnnotationMapping(const FunctionDecl *Func,
   return Result;
 }
 
+llvm::Error test::checkDataflowWithNoopAnalysis(
+    llvm::StringRef Code,
+    std::function<
+        void(const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &,
+             ASTContext &)>
+        VerifyResults,
+    DataflowAnalysisOptions Options, LangStandard::Kind Std,
+    llvm::StringRef TargetFun) {
+  using ast_matchers::hasName;
+  llvm::SmallVector<std::string, 3> ASTBuildArgs = {
+      // -fnodelayed-template-parsing is the default everywhere but on Windows.
+      // Set it explicitly so that tests behave the same on Windows as on other
+      // platforms.
+      "-fsyntax-only", "-fno-delayed-template-parsing",
+      "-std=" +
+          std::string(LangStandard::getLangStandardForKind(Std).getName())};
+  AnalysisInputs<NoopAnalysis> AI(
+      Code, hasName(TargetFun),
+      [UseBuiltinModel = Options.BuiltinOpts.has_value()](ASTContext &C,
+                                                          Environment &Env) {
+        return NoopAnalysis(
+            C,
+            DataflowAnalysisOptions{
+                UseBuiltinModel ? Env.getDataflowAnalysisContext().getOptions()
+                                : std::optional<BuiltinOptions>()});
+      });
+  AI.ASTBuildArgs = ASTBuildArgs;
+  if (Options.BuiltinOpts)
+    AI.BuiltinOptions = *Options.BuiltinOpts;
+  return checkDataflow<NoopAnalysis>(
+      std::move(AI),
+      /*VerifyResults=*/
+      [&VerifyResults](
+          const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
+          const AnalysisOutputs &AO) { VerifyResults(Results, AO.ASTCtx); });
+}
+
 const ValueDecl *test::findValueDecl(ASTContext &ASTCtx, llvm::StringRef Name) {
   auto TargetNodes = match(
       valueDecl(unless(indirectFieldDecl()), hasName(Name)).bind("v"), ASTCtx);

diff  --git a/clang/unittests/Analysis/FlowSensitive/TestingSupport.h b/clang/unittests/Analysis/FlowSensitive/TestingSupport.h
index 507fd5290aef52..93991d87d77f20 100644
--- a/clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ b/clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -387,40 +387,15 @@ using BuiltinOptions = DataflowAnalysisContext::Options;
 
 /// Runs dataflow on `Code` with a `NoopAnalysis` and calls `VerifyResults` to
 /// verify the results.
-template <typename VerifyResultsT>
-llvm::Error
-runDataflowReturnError(llvm::StringRef Code, VerifyResultsT VerifyResults,
-                       DataflowAnalysisOptions Options,
-                       LangStandard::Kind Std = LangStandard::lang_cxx17,
-                       llvm::StringRef TargetFun = "target") {
-  using ast_matchers::hasName;
-  llvm::SmallVector<std::string, 3> ASTBuildArgs = {
-      // -fnodelayed-template-parsing is the default everywhere but on Windows.
-      // Set it explicitly so that tests behave the same on Windows as on other
-      // platforms.
-      "-fsyntax-only", "-fno-delayed-template-parsing",
-      "-std=" +
-          std::string(LangStandard::getLangStandardForKind(Std).getName())};
-  AnalysisInputs<NoopAnalysis> AI(
-      Code, hasName(TargetFun),
-      [UseBuiltinModel = Options.BuiltinOpts.has_value()](ASTContext &C,
-                                                          Environment &Env) {
-        return NoopAnalysis(
-            C,
-            DataflowAnalysisOptions{
-                UseBuiltinModel ? Env.getDataflowAnalysisContext().getOptions()
-                                : std::optional<BuiltinOptions>()});
-      });
-  AI.ASTBuildArgs = ASTBuildArgs;
-  if (Options.BuiltinOpts)
-    AI.BuiltinOptions = *Options.BuiltinOpts;
-  return checkDataflow<NoopAnalysis>(
-      std::move(AI),
-      /*VerifyResults=*/
-      [&VerifyResults](
-          const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
-          const AnalysisOutputs &AO) { VerifyResults(Results, AO.ASTCtx); });
-}
+llvm::Error checkDataflowWithNoopAnalysis(
+    llvm::StringRef Code,
+    std::function<
+        void(const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &,
+             ASTContext &)>
+        VerifyResults,
+    DataflowAnalysisOptions Options,
+    LangStandard::Kind Std = LangStandard::lang_cxx17,
+    llvm::StringRef TargetFun = "target");
 
 /// Returns the `ValueDecl` for the given identifier.
 ///

diff  --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index 2ccd3e82baadc9..3f99ff5652ce28 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -38,21 +38,28 @@ using ::testing::Ne;
 using ::testing::NotNull;
 using ::testing::UnorderedElementsAre;
 
-template <typename VerifyResultsT>
-void runDataflow(llvm::StringRef Code, VerifyResultsT VerifyResults,
-                 DataflowAnalysisOptions Options,
-                 LangStandard::Kind Std = LangStandard::lang_cxx17,
-                 llvm::StringRef TargetFun = "target") {
-  ASSERT_THAT_ERROR(
-      runDataflowReturnError(Code, VerifyResults, Options, Std, TargetFun),
-      llvm::Succeeded());
-}
-
-template <typename VerifyResultsT>
-void runDataflow(llvm::StringRef Code, VerifyResultsT VerifyResults,
-                 LangStandard::Kind Std = LangStandard::lang_cxx17,
-                 bool ApplyBuiltinTransfer = true,
-                 llvm::StringRef TargetFun = "target") {
+void runDataflow(
+    llvm::StringRef Code,
+    std::function<
+        void(const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &,
+             ASTContext &)>
+        VerifyResults,
+    DataflowAnalysisOptions Options,
+    LangStandard::Kind Std = LangStandard::lang_cxx17,
+    llvm::StringRef TargetFun = "target") {
+  ASSERT_THAT_ERROR(checkDataflowWithNoopAnalysis(Code, VerifyResults, Options,
+                                                  Std, TargetFun),
+                    llvm::Succeeded());
+}
+
+void runDataflow(
+    llvm::StringRef Code,
+    std::function<
+        void(const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &,
+             ASTContext &)>
+        VerifyResults,
+    LangStandard::Kind Std = LangStandard::lang_cxx17,
+    bool ApplyBuiltinTransfer = true, llvm::StringRef TargetFun = "target") {
   runDataflow(Code, std::move(VerifyResults),
               {ApplyBuiltinTransfer ? BuiltinOptions{}
                                     : std::optional<BuiltinOptions>()},
@@ -2682,7 +2689,7 @@ TEST(TransferTest, CannotAnalyzeFunctionTemplate) {
     void target() {}
   )";
   ASSERT_THAT_ERROR(
-      runDataflowReturnError(
+      checkDataflowWithNoopAnalysis(
           Code,
           [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
              ASTContext &ASTCtx) {},
@@ -2698,7 +2705,7 @@ TEST(TransferTest, CannotAnalyzeMethodOfClassTemplate) {
     };
   )";
   ASSERT_THAT_ERROR(
-      runDataflowReturnError(
+      checkDataflowWithNoopAnalysis(
           Code,
           [](const llvm::StringMap<DataflowAnalysisState<NoopLattice>> &Results,
              ASTContext &ASTCtx) {},


        


More information about the cfe-commits mailing list