[llvm-branch-commits] [flang] [flang][OpenMP] Introduce WithSource<T> to couple T with source location (PR #190646)
via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Mon Apr 6 11:54:37 PDT 2026
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-flang-openmp
@llvm/pr-subscribers-flang-semantics
Author: Krzysztof Parzyszek (kparzysz)
<details>
<summary>Changes</summary>
The need for that has already happened once with SourcedActionStmt, and will happen again in upcoming PRs.
Issue: https://github.com/llvm/llvm-project/issues/185287
---
Full diff: https://github.com/llvm/llvm-project/pull/190646.diff
3 Files Affected:
- (modified) flang/include/flang/Semantics/openmp-utils.h (+21-5)
- (modified) flang/lib/Semantics/check-omp-atomic.cpp (+26-26)
- (modified) flang/lib/Semantics/check-omp-structure.cpp (+3-3)
``````````diff
diff --git a/flang/include/flang/Semantics/openmp-utils.h b/flang/include/flang/Semantics/openmp-utils.h
index 6a41ada9067c5..3084750bf73de 100644
--- a/flang/include/flang/Semantics/openmp-utils.h
+++ b/flang/include/flang/Semantics/openmp-utils.h
@@ -53,14 +53,30 @@ template <typename T> T &&AsRvalue(T &&t) { return std::move(t); }
const Scope &GetScopingUnit(const Scope &scope);
const Scope &GetProgramUnit(const Scope &scope);
+template <typename T> struct WithSource {
+ template <//
+ typename U = std::remove_reference_t<T>,
+ typename = std::enable_if_t<std::is_default_constructible_v<U>>>
+ WithSource() : value(), source() {}
+ WithSource(const WithSource<T> &) = default;
+ WithSource(WithSource<T> &&) = default;
+ WithSource(const T &t, parser::CharBlock s) : value(t), source(s) {}
+ WithSource(T &&t, parser::CharBlock s) : value(std::move(t)), source(s) {}
+ WithSource &operator=(const WithSource<T> &) = default;
+ WithSource &operator=(WithSource<T> &&) = default;
+
+ using value_type = T;
+ T value;
+ parser::CharBlock source;
+};
+
// There is no consistent way to get the source of an ActionStmt, but there
// is "source" in Statement<T>. This structure keeps the ActionStmt with the
// extracted source for further use.
-struct SourcedActionStmt {
- const parser::ActionStmt *stmt{nullptr};
- parser::CharBlock source;
-
- operator bool() const { return stmt != nullptr; }
+struct SourcedActionStmt : public WithSource<const parser::ActionStmt *> {
+ using WithSource<value_type>::WithSource;
+ value_type stmt() const { return value; }
+ operator bool() const { return stmt() != nullptr; }
};
SourcedActionStmt GetActionStmt(const parser::ExecutionPartConstruct *x);
diff --git a/flang/lib/Semantics/check-omp-atomic.cpp b/flang/lib/Semantics/check-omp-atomic.cpp
index effa3bf68063d..f722de8d8dc46 100644
--- a/flang/lib/Semantics/check-omp-atomic.cpp
+++ b/flang/lib/Semantics/check-omp-atomic.cpp
@@ -298,7 +298,7 @@ static std::optional<AnalyzedCondStmt> AnalyzeConditionalStmt(
if (auto &&action{GetActionStmt(x)}) {
if (auto *ifs{std::get_if<common::Indirection<parser::IfStmt>>(
- &action.stmt->u)}) {
+ &action.stmt()->u)}) {
const parser::IfStmt &s{ifs->value()};
auto &&maybeCond{
getFromLogical(std::get<parser::ScalarLogicalExpr>(s.t))};
@@ -335,13 +335,13 @@ static std::optional<AnalyzedCondStmt> AnalyzeConditionalStmt(
AnalyzedCondStmt result{std::move(*maybeCond), stmt.source,
GetActionStmt(std::get<parser::Block>(s.t)),
GetActionStmt(std::get<parser::Block>(maybeElse->t))};
- if (result.ift.stmt && result.iff.stmt) {
+ if (result.ift.stmt() && result.iff.stmt()) {
return result;
}
} else {
AnalyzedCondStmt result{std::move(*maybeCond), stmt.source,
GetActionStmt(std::get<parser::Block>(s.t)), SourcedActionStmt{}};
- if (result.ift.stmt) {
+ if (result.ift.stmt()) {
return result;
}
}
@@ -655,10 +655,10 @@ OmpStructureChecker::CheckUpdateCapture(
SourcedActionStmt act1{GetActionStmt(ec1)};
SourcedActionStmt act2{GetActionStmt(ec2)};
- auto maybeAssign1{GetEvaluateAssignment(act1.stmt)};
- auto maybeAssign2{GetEvaluateAssignment(act2.stmt)};
+ auto maybeAssign1{GetEvaluateAssignment(act1.stmt())};
+ auto maybeAssign2{GetEvaluateAssignment(act2.stmt())};
if (!maybeAssign1 || !maybeAssign2) {
- if (!IsAssignment(act1.stmt) || !IsAssignment(act2.stmt)) {
+ if (!IsAssignment(act1.stmt()) || !IsAssignment(act2.stmt())) {
context_.Say(source,
"ATOMIC UPDATE operation with CAPTURE should contain two assignments"_err_en_US);
}
@@ -1108,9 +1108,9 @@ void OmpStructureChecker::CheckAtomicConditionalUpdateStmt(
// - cond: associated(x, e) ift: x => expr iff: -
// The if-true statement must be present, and must be an assignment.
- auto maybeAssign{GetEvaluateAssignment(update.ift.stmt)};
+ auto maybeAssign{GetEvaluateAssignment(update.ift.stmt())};
if (!maybeAssign) {
- if (update.ift.stmt && !IsAssignment(update.ift.stmt)) {
+ if (update.ift.stmt() && !IsAssignment(update.ift.stmt())) {
context_.Say(update.ift.source,
"In ATOMIC UPDATE COMPARE the update statement should be an assignment"_err_en_US);
} else {
@@ -1138,7 +1138,7 @@ void OmpStructureChecker::CheckAtomicUpdateOnly(
parser::CharBlock source) {
if (body.size() == 1) {
SourcedActionStmt action{GetActionStmt(&body.front())};
- if (auto maybeUpdate{GetEvaluateAssignment(action.stmt)}) {
+ if (auto maybeUpdate{GetEvaluateAssignment(action.stmt())}) {
const SomeExpr &atom{maybeUpdate->lhs};
auto maybeAssign{
CheckAtomicUpdateAssignment(*maybeUpdate, action.source)};
@@ -1148,7 +1148,7 @@ void OmpStructureChecker::CheckAtomicUpdateOnly(
x.analysis = AtomicAnalysis(atom)
.addOp0(Analysis::Update, updateAssign)
.addOp1(Analysis::None);
- } else if (!IsAssignment(action.stmt)) {
+ } else if (!IsAssignment(action.stmt())) {
context_.Say(
source, "ATOMIC UPDATE operation should be an assignment"_err_en_US);
}
@@ -1197,7 +1197,7 @@ void OmpStructureChecker::CheckAtomicConditionalUpdate(
if (SourcedActionStmt action{GetActionStmt(cst)}) {
// The "condition" statement must be `r = cond`.
- if (auto maybeCond{GetEvaluateAssignment(action.stmt)}) {
+ if (auto maybeCond{GetEvaluateAssignment(action.stmt())}) {
if (maybeCond->lhs != update.cond) {
context_.Say(update.source,
"In ATOMIC UPDATE COMPARE the conditional statement must use %s as the condition"_err_en_US,
@@ -1213,7 +1213,7 @@ void OmpStructureChecker::CheckAtomicConditionalUpdate(
}
}
- evaluate::Assignment assign{*GetEvaluateAssignment(update.ift.stmt)};
+ evaluate::Assignment assign{*GetEvaluateAssignment(update.ift.stmt())};
CheckAtomicConditionalUpdateStmt(update, source);
if (IsCheckForAssociated(update.cond)) {
@@ -1254,8 +1254,8 @@ void OmpStructureChecker::CheckAtomicUpdateCapture(
SourcedActionStmt cact{GetActionStmt(cec)};
// The "dereferences" of std::optional are guaranteed to be valid after
// CheckUpdateCapture.
- evaluate::Assignment update{*GetEvaluateAssignment(uact.stmt)};
- evaluate::Assignment capture{*GetEvaluateAssignment(cact.stmt)};
+ evaluate::Assignment update{*GetEvaluateAssignment(uact.stmt())};
+ evaluate::Assignment capture{*GetEvaluateAssignment(cact.stmt())};
const SomeExpr &atom{update.lhs};
@@ -1280,7 +1280,7 @@ void OmpStructureChecker::CheckAtomicUpdateCapture(
return;
}
- if (GetActionStmt(&body.front()).stmt == uact.stmt) {
+ if (GetActionStmt(&body.front()).stmt() == uact.stmt()) {
x.analysis = AtomicAnalysis(atom)
.addOp0(action, updateAssign)
.addOp1(Analysis::Read, capture);
@@ -1316,7 +1316,7 @@ void OmpStructureChecker::CheckAtomicConditionalUpdateCapture(
auto classifyNonUpdate{[&](const SourcedActionStmt &action) {
// The non-update statement is either "r = cond" or the capture.
- if (auto maybeAssign{GetEvaluateAssignment(action.stmt)}) {
+ if (auto maybeAssign{GetEvaluateAssignment(action.stmt())}) {
if (update.cond == maybeAssign->lhs) {
// If this is "r = cond; if (r) ...", then update the condition.
update.cond = maybeAssign->rhs;
@@ -1384,10 +1384,10 @@ void OmpStructureChecker::CheckAtomicConditionalUpdateCapture(
}
// The update must have a form `x = d` or `x => d`.
- if (auto maybeWrite{GetEvaluateAssignment(update.ift.stmt)}) {
+ if (auto maybeWrite{GetEvaluateAssignment(update.ift.stmt())}) {
const SomeExpr &atom{maybeWrite->lhs};
CheckAtomicWriteAssignment(*maybeWrite, update.ift.source);
- if (auto maybeCapture{GetEvaluateAssignment(capture.stmt)}) {
+ if (auto maybeCapture{GetEvaluateAssignment(capture.stmt())}) {
CheckAtomicCaptureAssignment(*maybeCapture, atom, capture.source);
if (IsPointerAssignment(*maybeWrite) !=
@@ -1397,14 +1397,14 @@ void OmpStructureChecker::CheckAtomicConditionalUpdateCapture(
return;
}
} else {
- if (!IsAssignment(capture.stmt)) {
+ if (!IsAssignment(capture.stmt())) {
context_.Say(capture.source,
"In ATOMIC UPDATE COMPARE CAPTURE the capture statement should be an assignment"_err_en_US);
}
return;
}
} else {
- if (!IsAssignment(update.ift.stmt)) {
+ if (!IsAssignment(update.ift.stmt())) {
context_.Say(update.ift.source,
"In ATOMIC UPDATE COMPARE CAPTURE the update statement should be an assignment"_err_en_US);
}
@@ -1430,8 +1430,8 @@ void OmpStructureChecker::CheckAtomicConditionalUpdateCapture(
int updateWhen{!condUnused ? Analysis::IfTrue : 0};
int captureWhen{!captureAlways ? Analysis::IfFalse : 0};
- evaluate::Assignment updAssign{*GetEvaluateAssignment(update.ift.stmt)};
- evaluate::Assignment capAssign{*GetEvaluateAssignment(capture.stmt)};
+ evaluate::Assignment updAssign{*GetEvaluateAssignment(update.ift.stmt())};
+ evaluate::Assignment capAssign{*GetEvaluateAssignment(capture.stmt())};
const SomeExpr &atom{updAssign.lhs};
if (captureFirst) {
@@ -1465,7 +1465,7 @@ void OmpStructureChecker::CheckAtomicRead(
if (body.size() == 1) {
SourcedActionStmt action{GetActionStmt(&body.front())};
- if (auto maybeRead{GetEvaluateAssignment(action.stmt)}) {
+ if (auto maybeRead{GetEvaluateAssignment(action.stmt())}) {
CheckAtomicReadAssignment(*maybeRead, action.source);
if (auto maybe{GetConvertInput(maybeRead->rhs)}) {
@@ -1475,7 +1475,7 @@ void OmpStructureChecker::CheckAtomicRead(
.addOp0(Analysis::Read, maybeRead)
.addOp1(Analysis::None);
}
- } else if (!IsAssignment(action.stmt)) {
+ } else if (!IsAssignment(action.stmt())) {
context_.Say(
x.source, "ATOMIC READ operation should be an assignment"_err_en_US);
}
@@ -1500,7 +1500,7 @@ void OmpStructureChecker::CheckAtomicWrite(
if (body.size() == 1) {
SourcedActionStmt action{GetActionStmt(&body.front())};
- if (auto maybeWrite{GetEvaluateAssignment(action.stmt)}) {
+ if (auto maybeWrite{GetEvaluateAssignment(action.stmt())}) {
const SomeExpr &atom{maybeWrite->lhs};
CheckAtomicWriteAssignment(*maybeWrite, action.source);
@@ -1508,7 +1508,7 @@ void OmpStructureChecker::CheckAtomicWrite(
x.analysis = AtomicAnalysis(atom)
.addOp0(Analysis::Write, maybeWrite)
.addOp1(Analysis::None);
- } else if (!IsAssignment(action.stmt)) {
+ } else if (!IsAssignment(action.stmt())) {
context_.Say(
x.source, "ATOMIC WRITE operation should be an assignment"_err_en_US);
}
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 9b9da227bdef2..479f1763ee319 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -1754,7 +1754,7 @@ static std::pair<const parser::AllocateStmt *, parser::CharBlock>
getAllocateStmtAndSource(const parser::ExecutionPartConstruct *epc) {
if (SourcedActionStmt as{GetActionStmt(epc)}) {
using IndirectionAllocateStmt = common::Indirection<parser::AllocateStmt>;
- if (auto *indirect{std::get_if<IndirectionAllocateStmt>(&as.stmt->u)}) {
+ if (auto *indirect{std::get_if<IndirectionAllocateStmt>(&as.stmt()->u)}) {
return {&indirect->value(), as.source};
}
}
@@ -2258,11 +2258,11 @@ void OmpStructureChecker::Enter(const parser::OpenMPDispatchConstruct &x) {
bool passChecks{false};
omp::SourcedActionStmt action{omp::GetActionStmt(block)};
if (const auto *assignStmt{
- parser::Unwrap<parser::AssignmentStmt>(*action.stmt)}) {
+ parser::Unwrap<parser::AssignmentStmt>(*action.stmt())}) {
if (parser::Unwrap<parser::FunctionReference>(assignStmt->t)) {
passChecks = true;
}
- } else if (parser::Unwrap<parser::CallStmt>(*action.stmt)) {
+ } else if (parser::Unwrap<parser::CallStmt>(*action.stmt())) {
passChecks = true;
}
``````````
</details>
https://github.com/llvm/llvm-project/pull/190646
More information about the llvm-branch-commits
mailing list