[flang][OpenMP] move omp end directive validation to semantics #154739
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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).
llvmbot
added
flang
Member
|
@llvm/pr-subscribers-flang-parser Author: Tom Eccles (tblah) ChangesThe 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:
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
|
Member
|
@llvm/pr-subscribers-flang-openmp Author: Tom Eccles (tblah) ChangesThe 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:
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
|
Member
|
@llvm/pr-subscribers-flang-semantics Author: Tom Eccles (tblah) ChangesThe 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:
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
|
Contributor
Author
|
Part 2: #154740 |
kparzysz
reviewed
Aug 21, 2025
| // Common base class for block-associated constructs. | ||
| struct OmpBlockConstruct { | ||
| TUPLE_CLASS_BOILERPLATE(OmpBlockConstruct); | ||
| ENUM_CLASS(Flags, None, MissingMandatoryEndDirective); |
Contributor
There was a problem hiding this comment.
Do we need a flag for this? All loosely-structured blocks need the end-directive, and it's easy to tell whether a block is strictly- or loosely-structured (single BLOCK vs. sequence that doesn't start with BLOCK). I think we could simply have an optional end-directive in this case as well without the flag, and check the type of block in semantics.
Contributor
Author
There was a problem hiding this comment.
Thanks for the quick review. Good idea.
kparzysz
approved these changes
Aug 25, 2025
Contributor
kparzysz
left a comment
kparzysz
left a comment
There was a problem hiding this comment.
LGTM. Sorry I missed the update.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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).