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) {}, _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits