mboehme created this revision.
Herald added subscribers: martong, xazax.hun.
Herald added a reviewer: NoQ.
Herald added a project: All.
mboehme requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
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.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D154339
Files:
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
Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===================================================================
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -38,21 +38,28 @@
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") {
+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(
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,
+ LangStandard::Kind Std = LangStandard::lang_cxx17,
+ bool ApplyBuiltinTransfer = true, llvm::StringRef TargetFun = "target") {
runDataflow(Code, std::move(VerifyResults),
{ApplyBuiltinTransfer ? BuiltinOptions{}
: std::optional<BuiltinOptions>()},
Index: clang/unittests/Analysis/FlowSensitive/TestingSupport.h
===================================================================
--- clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -386,40 +386,15 @@
/// 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 runDataflowReturnError(
+ 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.
///
Index: clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp
===================================================================
--- clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp
+++ clang/unittests/Analysis/FlowSensitive/TestingSupport.cpp
@@ -153,6 +153,43 @@
return Result;
}
+llvm::Error test::runDataflowReturnError(
+ 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);
Index: clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp
===================================================================
--- clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/RecordOpsTest.cpp
@@ -16,10 +16,14 @@
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,
Index: clang/include/clang/Analysis/FlowSensitive/RecordOps.h
===================================================================
--- clang/include/clang/Analysis/FlowSensitive/RecordOps.h
+++ clang/include/clang/Analysis/FlowSensitive/RecordOps.h
@@ -23,7 +23,7 @@
///
/// 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 @@
/// 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.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
