[clang] [clang][dataflow] Allow `DataflowAnalysisContext` to use a non-owned `Solver`. (PR #91316)
martinboehme wrote: @joker-eph PS I don't see a revert of this PR, but I do see a revert of a different PR [here](https://github.com/llvm/llvm-project/commit/341aecc2dd0f6debcbe9f251a6d2e8a60d327eea), and that PR does look as if it could be the culprit. Maybe you commented on this PR by mistake? https://github.com/llvm/llvm-project/pull/91316 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Allow `DataflowAnalysisContext` to use a non-owned `Solver`. (PR #91316)
martinboehme wrote: > Actually your premerge failure here was an infra issue, but the CI is still > broken after this PR somehow: > https://lab.llvm.org/buildbot/#/builders/271/builds/7420 > > ``` > # .---command stderr > # | error: 'expected-error' diagnostics seen but not expected: > # | (frontend): '-fsyntax-only' action ignored; '-ast-print' action > specified previously > # `- > ``` This is a failure in the test clang\test\OpenMP\target_ast_print.cpp, correct? Are you sure this PR is the culprit? It touches only the dataflow framework, which is not used in OpenMP or the Clang compiler itself. https://github.com/llvm/llvm-project/pull/91316 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Allow `DataflowAnalysisContext` to use a non-owned `Solver`. (PR #91316)
joker-eph wrote: Actually your premerge failure here was an infra issue, but the CI is still broken after this PR somehow: https://lab.llvm.org/buildbot/#/builders/271/builds/7420 https://github.com/llvm/llvm-project/pull/91316 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Allow `DataflowAnalysisContext` to use a non-owned `Solver`. (PR #91316)
joker-eph wrote: CI failure wasn't unrelated actually: the CI is consistently broken after this patch (you have got an email about it?), I'll revert for now. https://github.com/llvm/llvm-project/pull/91316 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Allow `DataflowAnalysisContext` to use a non-owned `Solver`. (PR #91316)
https://github.com/martinboehme closed https://github.com/llvm/llvm-project/pull/91316 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Allow `DataflowAnalysisContext` to use a non-owned `Solver`. (PR #91316)
martinboehme wrote: CI failure looks unrelated. https://github.com/llvm/llvm-project/pull/91316 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Allow `DataflowAnalysisContext` to use a non-owned `Solver`. (PR #91316)
https://github.com/Xazax-hun approved this pull request. https://github.com/llvm/llvm-project/pull/91316 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Allow `DataflowAnalysisContext` to use a non-owned `Solver`. (PR #91316)
martinboehme wrote:
@ymand PTAL -- I noticed that my initial version had a use-after-move on this
line:
```cxx
DataflowAnalysisContext(std::unique_ptr S,
Options Opts = Options{
/*ContextSensitiveOpts=*/std::nullopt,
/*Logger=*/nullptr})
: DataflowAnalysisContext(*S, std::move(S), Opts) {}
```
This was because the private constructor to which this constructor delegates
originally took the `std::unique_ptr` by value, and the order in which
the arguments are evaluated is indeterminate.
I've now changed the parameter of the private constructor to be an rvalue
reference (`std::unique_ptr &&`), which resolves this issue as the move
only happens inside the constructor.
https://github.com/llvm/llvm-project/pull/91316
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Allow `DataflowAnalysisContext` to use a non-owned `Solver`. (PR #91316)
https://github.com/ymand approved this pull request. https://github.com/llvm/llvm-project/pull/91316 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Allow `DataflowAnalysisContext` to use a non-owned `Solver`. (PR #91316)
@@ -209,6 +221,9 @@ class DataflowAnalysisContext {
using DenseMapInfo::isEqual;
};
+ DataflowAnalysisContext(Solver , std::unique_ptr OwnedSolver,
martinboehme wrote:
Done.
https://github.com/llvm/llvm-project/pull/91316
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Allow `DataflowAnalysisContext` to use a non-owned `Solver`. (PR #91316)
https://github.com/martinboehme updated
https://github.com/llvm/llvm-project/pull/91316
>From abb7d53778394a220353deeeb86fbd06bf4352c2 Mon Sep 17 00:00:00 2001
From: Martin Braenne
Date: Tue, 7 May 2024 10:23:16 +
Subject: [PATCH 1/2] [clang][dataflow] Allow `DataflowAnalysisContext` to use
a non-owned `Solver`.
For some callers (see change in DataflowAnalysis.h), this is more convenient.
---
.../Analysis/FlowSensitive/DataflowAnalysis.h | 5 ++---
.../FlowSensitive/DataflowAnalysisContext.h | 20 +--
.../FlowSensitive/DataflowAnalysisContext.cpp | 10 +-
3 files changed, 25 insertions(+), 10 deletions(-)
diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
index 67eccdd030dcdd..763af244547647 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
@@ -283,9 +283,8 @@ llvm::Expected>
diagnoseFunction(
if (!Context)
return Context.takeError();
- auto OwnedSolver = std::make_unique(MaxSATIterations);
- const WatchedLiteralsSolver *Solver = OwnedSolver.get();
- DataflowAnalysisContext AnalysisContext(std::move(OwnedSolver));
+ auto Solver = std::make_unique(MaxSATIterations);
+ DataflowAnalysisContext AnalysisContext(*Solver);
Environment Env(AnalysisContext, FuncDecl);
AnalysisT Analysis = createAnalysis(ASTCtx, Env);
llvm::SmallVector Diagnostics;
diff --git
a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
index aa2c366cb164a9..13a74281e02b5d 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
@@ -67,7 +67,19 @@ class DataflowAnalysisContext {
DataflowAnalysisContext(std::unique_ptr S,
Options Opts = Options{
/*ContextSensitiveOpts=*/std::nullopt,
- /*Logger=*/nullptr});
+ /*Logger=*/nullptr})
+ : DataflowAnalysisContext(*S, std::move(S), Opts) {}
+
+ /// Constructs a dataflow analysis context.
+ ///
+ /// Requirements:
+ ///
+ /// `S` must outlive the `DataflowAnalysisContext`.
+ DataflowAnalysisContext(Solver , Options Opts = Options{
+ /*ContextSensitiveOpts=*/std::nullopt,
+ /*Logger=*/nullptr})
+ : DataflowAnalysisContext(S, nullptr, Opts) {}
+
~DataflowAnalysisContext();
/// Sets a callback that returns the names and types of the synthetic fields
@@ -209,6 +221,9 @@ class DataflowAnalysisContext {
using DenseMapInfo::isEqual;
};
+ DataflowAnalysisContext(Solver , std::unique_ptr OwnedSolver,
+ Options Opts);
+
// Extends the set of modeled field declarations.
void addModeledFields(const FieldSet );
@@ -232,7 +247,8 @@ class DataflowAnalysisContext {
Solver::Result::Status::Unsatisfiable;
}
- std::unique_ptr S;
+ Solver
+ std::unique_ptr OwnedSolver;
std::unique_ptr A;
// Maps from program declarations and statements to storage locations that
are
diff --git a/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
b/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
index e94fd39c45dc15..3041dcf6d500c5 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
@@ -170,7 +170,7 @@ DataflowAnalysisContext::joinFlowConditions(Atom FirstToken,
Solver::Result DataflowAnalysisContext::querySolver(
llvm::SetVector Constraints) {
- return S->solve(Constraints.getArrayRef());
+ return S.solve(Constraints.getArrayRef());
}
bool DataflowAnalysisContext::flowConditionImplies(Atom Token,
@@ -338,10 +338,10 @@ static std::unique_ptr
makeLoggerFromCommandLine() {
return Logger::html(std::move(StreamFactory));
}
-DataflowAnalysisContext::DataflowAnalysisContext(std::unique_ptr S,
- Options Opts)
-: S(std::move(S)), A(std::make_unique()), Opts(Opts) {
- assert(this->S != nullptr);
+DataflowAnalysisContext::DataflowAnalysisContext(
+Solver , std::unique_ptr OwnedSolver, Options Opts)
+: S(S), OwnedSolver(std::move(OwnedSolver)), A(std::make_unique()),
+ Opts(Opts) {
// If the -dataflow-log command-line flag was set, synthesize a logger.
// This is ugly but provides a uniform method for ad-hoc debugging dataflow-
// based tools.
>From 6112536c0f400c10b919532d2b171c37d0d435ac Mon Sep 17 00:00:00 2001
From: Martin Braenne
Date: Tue, 7 May 2024 12:53:16 +
Subject: [PATCH 2/2] fixup! [clang][dataflow] Allow `DataflowAnalysisContext`
to use a non-owned `Solver`.
---
[clang] [clang][dataflow] Allow `DataflowAnalysisContext` to use a non-owned `Solver`. (PR #91316)
@@ -209,6 +221,9 @@ class DataflowAnalysisContext {
using DenseMapInfo::isEqual;
};
+ DataflowAnalysisContext(Solver , std::unique_ptr OwnedSolver,
ymand wrote:
Please add comments either here or on the fields relating S and OwnedSolver.
https://github.com/llvm/llvm-project/pull/91316
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Allow `DataflowAnalysisContext` to use a non-owned `Solver`. (PR #91316)
https://github.com/ymand approved this pull request. https://github.com/llvm/llvm-project/pull/91316 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Allow `DataflowAnalysisContext` to use a non-owned `Solver`. (PR #91316)
https://github.com/ymand edited https://github.com/llvm/llvm-project/pull/91316 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Allow `DataflowAnalysisContext` to use a non-owned `Solver`. (PR #91316)
llvmbot wrote:
@llvm/pr-subscribers-clang
Author: None (martinboehme)
Changes
For some callers (see change in DataflowAnalysis.h), this is more convenient.
---
Full diff: https://github.com/llvm/llvm-project/pull/91316.diff
3 Files Affected:
- (modified) clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
(+2-3)
- (modified)
clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h (+18-2)
- (modified) clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
(+5-5)
``diff
diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
index 67eccdd030dcd..763af24454764 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
@@ -283,9 +283,8 @@ llvm::Expected>
diagnoseFunction(
if (!Context)
return Context.takeError();
- auto OwnedSolver = std::make_unique(MaxSATIterations);
- const WatchedLiteralsSolver *Solver = OwnedSolver.get();
- DataflowAnalysisContext AnalysisContext(std::move(OwnedSolver));
+ auto Solver = std::make_unique(MaxSATIterations);
+ DataflowAnalysisContext AnalysisContext(*Solver);
Environment Env(AnalysisContext, FuncDecl);
AnalysisT Analysis = createAnalysis(ASTCtx, Env);
llvm::SmallVector Diagnostics;
diff --git
a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
index aa2c366cb164a..13a74281e02b5 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
@@ -67,7 +67,19 @@ class DataflowAnalysisContext {
DataflowAnalysisContext(std::unique_ptr S,
Options Opts = Options{
/*ContextSensitiveOpts=*/std::nullopt,
- /*Logger=*/nullptr});
+ /*Logger=*/nullptr})
+ : DataflowAnalysisContext(*S, std::move(S), Opts) {}
+
+ /// Constructs a dataflow analysis context.
+ ///
+ /// Requirements:
+ ///
+ /// `S` must outlive the `DataflowAnalysisContext`.
+ DataflowAnalysisContext(Solver , Options Opts = Options{
+ /*ContextSensitiveOpts=*/std::nullopt,
+ /*Logger=*/nullptr})
+ : DataflowAnalysisContext(S, nullptr, Opts) {}
+
~DataflowAnalysisContext();
/// Sets a callback that returns the names and types of the synthetic fields
@@ -209,6 +221,9 @@ class DataflowAnalysisContext {
using DenseMapInfo::isEqual;
};
+ DataflowAnalysisContext(Solver , std::unique_ptr OwnedSolver,
+ Options Opts);
+
// Extends the set of modeled field declarations.
void addModeledFields(const FieldSet );
@@ -232,7 +247,8 @@ class DataflowAnalysisContext {
Solver::Result::Status::Unsatisfiable;
}
- std::unique_ptr S;
+ Solver
+ std::unique_ptr OwnedSolver;
std::unique_ptr A;
// Maps from program declarations and statements to storage locations that
are
diff --git a/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
b/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
index e94fd39c45dc1..3041dcf6d500c 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
@@ -170,7 +170,7 @@ DataflowAnalysisContext::joinFlowConditions(Atom FirstToken,
Solver::Result DataflowAnalysisContext::querySolver(
llvm::SetVector Constraints) {
- return S->solve(Constraints.getArrayRef());
+ return S.solve(Constraints.getArrayRef());
}
bool DataflowAnalysisContext::flowConditionImplies(Atom Token,
@@ -338,10 +338,10 @@ static std::unique_ptr
makeLoggerFromCommandLine() {
return Logger::html(std::move(StreamFactory));
}
-DataflowAnalysisContext::DataflowAnalysisContext(std::unique_ptr S,
- Options Opts)
-: S(std::move(S)), A(std::make_unique()), Opts(Opts) {
- assert(this->S != nullptr);
+DataflowAnalysisContext::DataflowAnalysisContext(
+Solver , std::unique_ptr OwnedSolver, Options Opts)
+: S(S), OwnedSolver(std::move(OwnedSolver)), A(std::make_unique()),
+ Opts(Opts) {
// If the -dataflow-log command-line flag was set, synthesize a logger.
// This is ugly but provides a uniform method for ad-hoc debugging dataflow-
// based tools.
``
https://github.com/llvm/llvm-project/pull/91316
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][dataflow] Allow `DataflowAnalysisContext` to use a non-owned `Solver`. (PR #91316)
https://github.com/martinboehme created
https://github.com/llvm/llvm-project/pull/91316
For some callers (see change in DataflowAnalysis.h), this is more convenient.
>From abb7d53778394a220353deeeb86fbd06bf4352c2 Mon Sep 17 00:00:00 2001
From: Martin Braenne
Date: Tue, 7 May 2024 10:23:16 +
Subject: [PATCH] [clang][dataflow] Allow `DataflowAnalysisContext` to use a
non-owned `Solver`.
For some callers (see change in DataflowAnalysis.h), this is more convenient.
---
.../Analysis/FlowSensitive/DataflowAnalysis.h | 5 ++---
.../FlowSensitive/DataflowAnalysisContext.h | 20 +--
.../FlowSensitive/DataflowAnalysisContext.cpp | 10 +-
3 files changed, 25 insertions(+), 10 deletions(-)
diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
index 67eccdd030dcd..763af24454764 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
@@ -283,9 +283,8 @@ llvm::Expected>
diagnoseFunction(
if (!Context)
return Context.takeError();
- auto OwnedSolver = std::make_unique(MaxSATIterations);
- const WatchedLiteralsSolver *Solver = OwnedSolver.get();
- DataflowAnalysisContext AnalysisContext(std::move(OwnedSolver));
+ auto Solver = std::make_unique(MaxSATIterations);
+ DataflowAnalysisContext AnalysisContext(*Solver);
Environment Env(AnalysisContext, FuncDecl);
AnalysisT Analysis = createAnalysis(ASTCtx, Env);
llvm::SmallVector Diagnostics;
diff --git
a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
index aa2c366cb164a..13a74281e02b5 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
@@ -67,7 +67,19 @@ class DataflowAnalysisContext {
DataflowAnalysisContext(std::unique_ptr S,
Options Opts = Options{
/*ContextSensitiveOpts=*/std::nullopt,
- /*Logger=*/nullptr});
+ /*Logger=*/nullptr})
+ : DataflowAnalysisContext(*S, std::move(S), Opts) {}
+
+ /// Constructs a dataflow analysis context.
+ ///
+ /// Requirements:
+ ///
+ /// `S` must outlive the `DataflowAnalysisContext`.
+ DataflowAnalysisContext(Solver , Options Opts = Options{
+ /*ContextSensitiveOpts=*/std::nullopt,
+ /*Logger=*/nullptr})
+ : DataflowAnalysisContext(S, nullptr, Opts) {}
+
~DataflowAnalysisContext();
/// Sets a callback that returns the names and types of the synthetic fields
@@ -209,6 +221,9 @@ class DataflowAnalysisContext {
using DenseMapInfo::isEqual;
};
+ DataflowAnalysisContext(Solver , std::unique_ptr OwnedSolver,
+ Options Opts);
+
// Extends the set of modeled field declarations.
void addModeledFields(const FieldSet );
@@ -232,7 +247,8 @@ class DataflowAnalysisContext {
Solver::Result::Status::Unsatisfiable;
}
- std::unique_ptr S;
+ Solver
+ std::unique_ptr OwnedSolver;
std::unique_ptr A;
// Maps from program declarations and statements to storage locations that
are
diff --git a/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
b/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
index e94fd39c45dc1..3041dcf6d500c 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
@@ -170,7 +170,7 @@ DataflowAnalysisContext::joinFlowConditions(Atom FirstToken,
Solver::Result DataflowAnalysisContext::querySolver(
llvm::SetVector Constraints) {
- return S->solve(Constraints.getArrayRef());
+ return S.solve(Constraints.getArrayRef());
}
bool DataflowAnalysisContext::flowConditionImplies(Atom Token,
@@ -338,10 +338,10 @@ static std::unique_ptr
makeLoggerFromCommandLine() {
return Logger::html(std::move(StreamFactory));
}
-DataflowAnalysisContext::DataflowAnalysisContext(std::unique_ptr S,
- Options Opts)
-: S(std::move(S)), A(std::make_unique()), Opts(Opts) {
- assert(this->S != nullptr);
+DataflowAnalysisContext::DataflowAnalysisContext(
+Solver , std::unique_ptr OwnedSolver, Options Opts)
+: S(S), OwnedSolver(std::move(OwnedSolver)), A(std::make_unique()),
+ Opts(Opts) {
// If the -dataflow-log command-line flag was set, synthesize a logger.
// This is ugly but provides a uniform method for ad-hoc debugging dataflow-
// based tools.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
