[flang-commits] [flang] [flang][OpenMP] move omp end directive validation to semantics (PR #154739)
via flang-commits
flang-commits at lists.llvm.org
Thu Aug 21 04:57:09 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-flang-parser
Author: Tom Eccles (tblah)
<details>
<summary>Changes</summary>
The old parse tree errors quckly exploded to thousands of unhelpful lines when there were multiple missing end directives (e.g. #<!-- -->90452).
Instead I've added a flag to the parse tree indicating when a missing end directive needs to be diagnosed, and moved the error messages to semantics (where they are a lot easier to control).
This has the disadvantage of not displaying the error if there were other parse errors, but there is a precedent for this approach (e.g. parsing atomic constructs).
---
Full diff: https://github.com/llvm/llvm-project/pull/154739.diff
7 Files Affected:
- (modified) flang/include/flang/Parser/dump-parse-tree.h (+1)
- (modified) flang/include/flang/Parser/parse-tree.h (+11-1)
- (modified) flang/lib/Parser/openmp-parsers.cpp (+20-4)
- (modified) flang/lib/Semantics/check-omp-structure.cpp (+8)
- (modified) flang/test/Parser/OpenMP/fail-construct1.f90 (+1-1)
- (added) flang/test/Parser/OpenMP/ordered-block-vs-standalone.f90 (+60)
- (added) flang/test/Semantics/OpenMP/missing-end-directive.f90 (+13)
``````````diff
diff --git a/flang/include/flang/Parser/dump-parse-tree.h b/flang/include/flang/Parser/dump-parse-tree.h
index 57421da4fa938..6bce8e8e5c00d 100644
--- a/flang/include/flang/Parser/dump-parse-tree.h
+++ b/flang/include/flang/Parser/dump-parse-tree.h
@@ -713,6 +713,7 @@ class ParseTreeDumper {
NODE(parser, OmpEndDirective)
NODE(parser, OpenMPAtomicConstruct)
NODE(parser, OpenMPBlockConstruct)
+ NODE_ENUM(OpenMPBlockConstruct, Flags)
NODE(parser, OpenMPCancelConstruct)
NODE(parser, OpenMPCancellationPointConstruct)
NODE(parser, OpenMPConstruct)
diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index 1d1a4a163084b..38ec605574c06 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -4783,15 +4783,25 @@ struct OmpEndDirective : public OmpDirectiveSpecification {
// Common base class for block-associated constructs.
struct OmpBlockConstruct {
TUPLE_CLASS_BOILERPLATE(OmpBlockConstruct);
+ ENUM_CLASS(Flags, None, MissingMandatoryEndDirective);
+
+ /// Constructor with defualt value for Flags.
+ OmpBlockConstruct(OmpBeginDirective &&begin, Block &&block,
+ std::optional<OmpEndDirective> &&end)
+ : t(std::move(begin), std::move(block), std::move(end), Flags::None) {}
+
const OmpBeginDirective &BeginDir() const {
return std::get<OmpBeginDirective>(t);
}
const std::optional<OmpEndDirective> &EndDir() const {
return std::get<std::optional<OmpEndDirective>>(t);
}
+ bool isMissingMandatoryEndDirecitive() const {
+ return std::get<Flags>(t) == Flags::MissingMandatoryEndDirective;
+ }
CharBlock source;
- std::tuple<OmpBeginDirective, Block, std::optional<OmpEndDirective>> t;
+ std::tuple<OmpBeginDirective, Block, std::optional<OmpEndDirective>, Flags> t;
};
struct OmpMetadirectiveDirective {
diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp
index 56cee4ab38e9b..9670302c8549b 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -1474,6 +1474,7 @@ struct OmpBlockConstructParser {
constexpr OmpBlockConstructParser(llvm::omp::Directive dir) : dir_(dir) {}
std::optional<resultType> Parse(ParseState &state) const {
+ using Flags = OmpBlockConstruct::Flags;
if (auto &&begin{OmpBeginDirectiveParser(dir_).Parse(state)}) {
if (auto &&body{attempt(StrictlyStructuredBlockParser{}).Parse(state)}) {
// Try strictly-structured block with an optional end-directive
@@ -1484,11 +1485,26 @@ struct OmpBlockConstructParser {
[](auto &&s) { return OmpEndDirective(std::move(s)); })};
} else if (auto &&body{
attempt(LooselyStructuredBlockParser{}).Parse(state)}) {
- // Try loosely-structured block with a mandatory end-directive
- if (auto end{OmpEndDirectiveParser{dir_}.Parse(state)}) {
- return OmpBlockConstruct{OmpBeginDirective(std::move(*begin)),
- std::move(*body), OmpEndDirective{std::move(*end)}};
+ // Try loosely-structured block with a mandatory end-directive.
+ auto end{maybe(OmpEndDirectiveParser{dir_}).Parse(state)};
+ // Dereference outer optional (maybe() always succeeds) and look at the
+ // inner optional.
+ bool endPresent = end->has_value();
+
+ // ORDERED is special. We do need to return failure here so that the
+ // standalone ORDERED construct can be distinguished from the block
+ // associated construct.
+ if (!endPresent && dir_ == llvm::omp::Directive::OMPD_ordered) {
+ return std::nullopt;
}
+
+ // Delay the error for a missing end-directive until semantics so that
+ // we have better control over the output.
+ return OmpBlockConstruct{OmpBeginDirective(std::move(*begin)),
+ std::move(*body),
+ llvm::transformOptional(std::move(*end),
+ [](auto &&s) { return OmpEndDirective(std::move(s)); }),
+ endPresent ? Flags::None : Flags::MissingMandatoryEndDirective};
}
}
return std::nullopt;
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 92a2cfc330d35..0bdd2c62f88ce 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -785,6 +785,14 @@ void OmpStructureChecker::Enter(const parser::OpenMPBlockConstruct &x) {
const parser::Block &block{std::get<parser::Block>(x.t)};
PushContextAndClauseSets(beginSpec.DirName().source, beginSpec.DirId());
+
+ // Missing mandatory end block: this is checked in semantics because that
+ // makes it easier to control the error messages.
+ if (x.isMissingMandatoryEndDirecitive()) {
+ context_.Say(
+ x.BeginDir().source, "Expected OpenMP end directive"_err_en_US);
+ }
+
if (llvm::omp::allTargetSet.test(GetContext().directive)) {
EnterDirectiveNest(TargetNest);
}
diff --git a/flang/test/Parser/OpenMP/fail-construct1.f90 b/flang/test/Parser/OpenMP/fail-construct1.f90
index f0b3f7438ae58..9d1af903344d3 100644
--- a/flang/test/Parser/OpenMP/fail-construct1.f90
+++ b/flang/test/Parser/OpenMP/fail-construct1.f90
@@ -1,5 +1,5 @@
! RUN: not %flang_fc1 -fsyntax-only -fopenmp %s 2>&1 | FileCheck %s
!$omp parallel
-! CHECK: error: expected '!$OMP '
+! CHECK: error: Expected OpenMP end directive
end
diff --git a/flang/test/Parser/OpenMP/ordered-block-vs-standalone.f90 b/flang/test/Parser/OpenMP/ordered-block-vs-standalone.f90
new file mode 100644
index 0000000000000..db9c45add8674
--- /dev/null
+++ b/flang/test/Parser/OpenMP/ordered-block-vs-standalone.f90
@@ -0,0 +1,60 @@
+! RUN: %flang_fc1 -fdebug-dump-parse-tree -fopenmp -fopenmp-version=45 %s | FileCheck %s
+
+! Check that standalone ORDERED is successfully distinguished form block associated ORDERED
+
+! CHECK: | SubroutineStmt
+! CHECK-NEXT: | | Name = 'standalone'
+subroutine standalone
+ integer :: x(10, 10)
+ do i = 1, 10
+ do j = 1,10
+ ! CHECK: OpenMPConstruct -> OpenMPStandaloneConstruct
+ ! CHECK-NEXT: | OmpDirectiveName -> llvm::omp::Directive = ordered
+ ! CHECK-NEXT: | OmpClauseList ->
+ ! CHECK-NEXT: | Flags = None
+ !$omp ordered
+ x(i, j) = i + j
+ end do
+ end do
+endsubroutine
+
+! CHECK: | SubroutineStmt
+! CHECK-NEXT: | | Name = 'strict_block'
+subroutine strict_block
+ integer :: x(10, 10)
+ integer :: tmp
+ do i = 1, 10
+ do j = 1,10
+ ! CHECK: OpenMPConstruct -> OpenMPBlockConstruct
+ ! CHECK-NEXT: | OmpBeginDirective
+ ! CHECK-NEXT: | | OmpDirectiveName -> llvm::omp::Directive = ordered
+ ! CHECK-NEXT: | | OmpClauseList ->
+ ! CHECK-NEXT: | | Flags = None
+ !$omp ordered
+ block
+ tmp = i + j
+ x(i, j) = tmp
+ end block
+ end do
+ end do
+endsubroutine
+
+! CHECK: | SubroutineStmt
+! CHECK-NEXT: | | Name = 'loose_block'
+subroutine loose_block
+ integer :: x(10, 10)
+ integer :: tmp
+ do i = 1, 10
+ do j = 1,10
+ ! CHECK: OpenMPConstruct -> OpenMPBlockConstruct
+ ! CHECK-NEXT: | OmpBeginDirective
+ ! CHECK-NEXT: | | OmpDirectiveName -> llvm::omp::Directive = ordered
+ ! CHECK-NEXT: | | OmpClauseList ->
+ ! CHECK-NEXT: | | Flags = None
+ !$omp ordered
+ tmp = i + j
+ x(i, j) = tmp
+ !$omp end ordered
+ end do
+ end do
+endsubroutine
diff --git a/flang/test/Semantics/OpenMP/missing-end-directive.f90 b/flang/test/Semantics/OpenMP/missing-end-directive.f90
new file mode 100644
index 0000000000000..3b870d134155b
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/missing-end-directive.f90
@@ -0,0 +1,13 @@
+! RUN: %python %S/../test_errors.py %s %flang -fopenmp
+
+! Test that we can diagnose missing end directives without an explosion of errors
+
+! ERROR: Expected OpenMP end directive
+!$omp parallel
+! ERROR: Expected OpenMP end directive
+!$omp task
+! ERROR: Expected OpenMP end directive
+!$omp parallel
+! ERROR: Expected OpenMP end directive
+!$omp task
+end
``````````
</details>
https://github.com/llvm/llvm-project/pull/154739
More information about the flang-commits
mailing list