[flang-commits] [flang] [flang][OpenMP] Allow parsing ODS as directive-specification list item (PR #185737)
via flang-commits
flang-commits at lists.llvm.org
Tue Mar 10 12:46:43 PDT 2026
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-flang-parser
@llvm/pr-subscribers-flang-openmp
Author: Krzysztof Parzyszek (kparzysz)
<details>
<summary>Changes</summary>
Normally a directive specification may use commas between the directive name and the clauses, and between the clauses. There are some instances, however, when a directive-specification is treated as a list item. Specifically in arguments to the APPLY clause and as an argument to WHEN, OTHERWISE, and the now-deprecated DEFAULT when used on a METADIRECTIVE. In those cases, use of commas is prohibited to avoid confusion between commas being part of the directive-specification, and the argument list separators.
---
Full diff: https://github.com/llvm/llvm-project/pull/185737.diff
2 Files Affected:
- (modified) flang/lib/Parser/openmp-parsers.cpp (+122-61)
- (added) flang/test/Parser/OpenMP/no-commas-in-ods-list-item.f90 (+16)
``````````diff
diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp
index 7838173d791a1..d335584cb8533 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -148,7 +148,7 @@ template <typename Parser> constexpr auto unwrap(const Parser &p) {
// parser.
// The intended use is with the "checker" parser being some token, followed
// by a more complex parser that consumes the token plus more things, e.g.
-// "PARALLEL"_id >= Parser<OmpDirectiveSpecification>{}.
+// "PARALLEL"_id >= OmpDirectiveSpecificationParser{}.
//
// The >= has a higher precedence than ||, so it can be used just like >>
// in an alternatives parser without parentheses.
@@ -1091,6 +1091,19 @@ TYPE_PARSER(construct<OmpAdjustArgsClause::OmpAdjustOp>(
// --- Parsers for clauses --------------------------------------------
+// Declaration of the ODS parser. This type must be complete for some of
+// the clause parsers.
+struct OmpDirectiveSpecificationParser {
+ using resultType = OmpDirectiveSpecification;
+ constexpr OmpDirectiveSpecificationParser(bool allowCommas = true)
+ : allowCommas_(allowCommas) {}
+
+ std::optional<resultType> Parse(ParseState &state) const;
+
+private:
+ const bool allowCommas_;
+};
+
/// `MOBClause` is a clause that has a
/// std::tuple<Modifiers, OmpObjectList, bool>.
/// Helper function to create a typical modifiers-objects clause, where the
@@ -1145,7 +1158,8 @@ TYPE_PARSER(construct<OmpDefaultClause::DataSharingAttribute>(
TYPE_PARSER(construct<OmpDefaultClause>(
construct<OmpDefaultClause>(
Parser<OmpDefaultClause::DataSharingAttribute>{}) ||
- construct<OmpDefaultClause>(indirect(Parser<OmpDirectiveSpecification>{}))))
+ construct<OmpDefaultClause>(
+ indirect(OmpDirectiveSpecificationParser(/*allowCommas=*/false)))))
TYPE_PARSER(construct<OmpDynGroupprivateClause>(
maybe(nonemptyList(Parser<OmpDynGroupprivateClause::Modifier>{}) / ":"),
@@ -1354,12 +1368,12 @@ TYPE_PARSER(construct<OmpMatchClause>(
Parser<traits::OmpContextSelectorSpecification>{}))
TYPE_PARSER(construct<OmpOtherwiseClause>(
- maybe(indirect(Parser<OmpDirectiveSpecification>{}))))
+ maybe(indirect(OmpDirectiveSpecificationParser(/*allowCommas=*/false)))))
TYPE_PARSER(construct<OmpWhenClause>(
maybe(nonemptyList(Parser<OmpWhenClause::Modifier>{}) / ":"),
- maybe(indirect(
- OmpStylizedInstanceCreator(Parser<OmpDirectiveSpecification>{})))))
+ maybe(indirect(OmpStylizedInstanceCreator(
+ OmpDirectiveSpecificationParser(/*allowCommas=*/false))))))
// OMP 5.2 12.6.1 grainsize([ prescriptiveness :] scalar-integer-expression)
TYPE_PARSER(construct<OmpGrainsizeClause>(
@@ -1644,9 +1658,34 @@ TYPE_PARSER( //
construct<OmpClause>(construct<OmpClause::CancellationConstructType>(
Parser<OmpCancellationConstructTypeClause>{})))
-// [Clause, [Clause], ...]
-TYPE_PARSER(sourced(construct<OmpClauseList>(
- many(maybe(","_tok) >> sourced(Parser<OmpClause>{})))))
+// If allowCommas is true:
+// [[,] OmpClause] ...
+// otherwise
+// [OmpClause] ...
+struct OmpClauseListParser {
+ using resultType = OmpClauseList;
+
+ constexpr OmpClauseListParser(bool allowCommas = true)
+ : allowCommas_(allowCommas) {}
+
+ std::optional<resultType> Parse(ParseState &state) const {
+ if (allowCommas_) {
+ auto &&p{many(maybe(","_tok) >> sourced(Parser<OmpClause>{}))};
+ return sourced(construct<OmpClauseList>(std::move(p))).Parse(state);
+ } else {
+ auto &&p{many(sourced(Parser<OmpClause>{}))};
+ return sourced(construct<OmpClauseList>(std::move(p))).Parse(state);
+ }
+ }
+
+private:
+ const bool allowCommas_;
+};
+
+// Prevent accidental use of Parser<OmpClauseList>.
+template <>
+auto Parser<OmpClauseList>::Parse(ParseState &state)
+ -> std::optional<OmpClauseList> = delete;
// 2.1 (variable | /common-block/ | array-sections)
TYPE_PARSER(construct<OmpObjectList>(nonemptyList(Parser<OmpObject>{})))
@@ -1673,6 +1712,12 @@ constexpr auto validBlock{many(validEPC)};
TYPE_PARSER(sourced(construct<OmpDirectiveName>(OmpDirectiveNameParser{})))
+// Prevent accidental use of Parser<OmpDirectiveSpecification>.
+// Use OmpDirectiveSpecificationParser instead.
+template <>
+auto Parser<OmpDirectiveSpecification>::Parse(ParseState &)
+ -> std::optional<OmpDirectiveSpecification> = delete;
+
OmpDirectiveSpecification static makeFlushFromOldSyntax(Verbatim &&text,
std::optional<OmpClauseList> &&clauses,
std::optional<OmpArgumentList> &&args,
@@ -1681,37 +1726,53 @@ OmpDirectiveSpecification static makeFlushFromOldSyntax(Verbatim &&text,
std::move(clauses), std::move(flags)};
}
-TYPE_PARSER(
- // Parse the old syntax: FLUSH [clauses] [(objects)]
- sourced(construct<OmpDirectiveSpecification>(
- // Force this old-syntax parser to fail for FLUSH followed by '('.
- // Otherwise it could succeed on the new syntax but have one of
- // lists absent in the parsed result.
- // E.g. for FLUSH(x) SEQ_CST it would find no clauses following
- // the directive name, parse the argument list "(x)" and stop.
- applyFunction<OmpDirectiveSpecification>(makeFlushFromOldSyntax,
- verbatim("FLUSH"_tok) / !lookAhead("("_tok),
- maybe(Parser<OmpClauseList>{}),
- maybe(parenthesized(
- OmpArgumentListParser<llvm::omp::Directive::OMPD_flush>{})),
- pure(OmpDirectiveSpecification::Flags(
- {OmpDirectiveSpecification::Flag::DeprecatedSyntax}))))) ||
- // Parse DECLARE_VARIANT individually, because the "[base:]variant"
- // argument will conflict with DECLARE_REDUCTION's "ident:types...".
- predicated(Parser<OmpDirectiveName>{},
- IsDirective(llvm::omp::Directive::OMPD_declare_variant)) >=
- sourced(construct<OmpDirectiveSpecification>(
- sourced(OmpDirectiveNameParser{}),
- maybe(parenthesized(OmpArgumentListParser<
- llvm::omp::Directive::OMPD_declare_variant>{})),
- maybe(Parser<OmpClauseList>{}),
- pure(OmpDirectiveSpecification::Flags()))) ||
- // Parse the standard syntax: directive [(arguments)] [clauses]
- sourced(construct<OmpDirectiveSpecification>( //
- sourced(OmpDirectiveNameParser{}),
- maybe(parenthesized(OmpArgumentListParser<>{})),
- maybe(Parser<OmpClauseList>{}),
- pure(OmpDirectiveSpecification::Flags()))))
+auto OmpDirectiveSpecificationParser::Parse(ParseState &state) const
+ -> std::optional<resultType> {
+ // Force this old-syntax parser to fail for FLUSH followed by '('.
+ // Otherwise it could succeed on the new syntax but have one of
+ // lists absent in the parsed result.
+ // E.g. for FLUSH(x) SEQ_CST it would find no clauses following
+ // the directive name, parse the argument list "(x)" and stop.
+ auto &&fp{//
+ applyFunction<OmpDirectiveSpecification>(makeFlushFromOldSyntax,
+ verbatim("FLUSH"_tok) / !lookAhead("("_tok),
+ maybe(OmpClauseListParser(allowCommas_)),
+ maybe(parenthesized(
+ OmpArgumentListParser<llvm::omp::Directive::OMPD_flush>{})),
+ pure(OmpDirectiveSpecification::Flags(
+ {OmpDirectiveSpecification::Flag::DeprecatedSyntax})))};
+ if (auto &&ods{attempt(sourced(fp)).Parse(state)}) {
+ return std::move(ods);
+ }
+
+ // Parse DECLARE_VARIANT individually, because the "[base:]variant"
+ // argument will conflict with DECLARE_REDUCTION's "ident:types...".
+ auto &&dvp{//
+ predicated(Parser<OmpDirectiveName>{},
+ IsDirective(llvm::omp::Directive::OMPD_declare_variant)) >=
+ sourced(construct<OmpDirectiveSpecification>(
+ sourced(OmpDirectiveNameParser{}),
+ maybe(parenthesized(OmpArgumentListParser<
+ llvm::omp::Directive::OMPD_declare_variant>{})),
+ maybe(OmpClauseListParser(allowCommas_)),
+ pure(OmpDirectiveSpecification::Flags())))};
+ if (auto &&ods{attempt(dvp).Parse(state)}) {
+ return std::move(ods);
+ }
+
+ // Parse the standard syntax: directive [(arguments)] [clauses]
+ auto &&odsp{//
+ sourced(construct<OmpDirectiveSpecification>( //
+ sourced(OmpDirectiveNameParser{}),
+ maybe(parenthesized(OmpArgumentListParser<>{})),
+ maybe(OmpClauseListParser(allowCommas_)),
+ pure(OmpDirectiveSpecification::Flags())))};
+ if (auto &&ods{attempt(odsp).Parse(state)}) {
+ return std::move(ods);
+ }
+
+ return std::nullopt;
+}
static bool IsStandaloneOrdered(const OmpDirectiveSpecification &dirSpec) {
// An ORDERED construct is standalone if it has DOACROSS or DEPEND clause.
@@ -1840,12 +1901,12 @@ struct LoopNestParser {
TYPE_PARSER(construct<OmpErrorDirective>(
predicated(Parser<OmpDirectiveName>{},
IsDirective(llvm::omp::Directive::OMPD_error)) >=
- Parser<OmpDirectiveSpecification>{}))
+ OmpDirectiveSpecificationParser{}))
TYPE_PARSER(construct<OmpNothingDirective>(
predicated(Parser<OmpDirectiveName>{},
IsDirective(llvm::omp::Directive::OMPD_nothing)) >=
- Parser<OmpDirectiveSpecification>{}))
+ OmpDirectiveSpecificationParser{}))
TYPE_PARSER( //
sourced(construct<OpenMPUtilityConstruct>(Parser<OmpErrorDirective>{})) ||
@@ -1854,7 +1915,7 @@ TYPE_PARSER( //
TYPE_PARSER(construct<OmpMetadirectiveDirective>(
predicated(Parser<OmpDirectiveName>{},
IsDirective(llvm::omp::Directive::OMPD_metadirective)) >=
- Parser<OmpDirectiveSpecification>{}))
+ OmpDirectiveSpecificationParser{}))
struct OmpBeginDirectiveParser {
using resultType = OmpDirectiveSpecification;
@@ -1866,7 +1927,7 @@ struct OmpBeginDirectiveParser {
std::optional<resultType> Parse(ParseState &state) const {
auto &&p{predicated(Parser<OmpDirectiveName>{}, IsMemberOf(dirs_)) >=
- Parser<OmpDirectiveSpecification>{}};
+ OmpDirectiveSpecificationParser{}};
return p.Parse(state);
}
@@ -2085,7 +2146,7 @@ struct OmpAtomicConstructParser {
}
recursing_ = true;
- auto dirSpec{Parser<OmpDirectiveSpecification>{}.Parse(state)};
+ auto dirSpec{OmpDirectiveSpecificationParser{}.Parse(state)};
if (!dirSpec || dirSpec->DirId() != llvm::omp::Directive::OMPD_atomic) {
recursing_ = false;
return std::nullopt;
@@ -2188,42 +2249,42 @@ static bool IsSimpleStandalone(const OmpDirectiveName &name) {
TYPE_PARSER(sourced( //
construct<OpenMPSimpleStandaloneConstruct>(
predicated(OmpDirectiveNameParser{}, IsSimpleStandalone) >=
- Parser<OmpDirectiveSpecification>{}) ||
+ OmpDirectiveSpecificationParser{}) ||
construct<OpenMPSimpleStandaloneConstruct>(
- predicated(Parser<OmpDirectiveSpecification>{}, IsStandaloneOrdered))))
+ predicated(OmpDirectiveSpecificationParser{}, IsStandaloneOrdered))))
TYPE_PARSER(sourced( //
construct<OpenMPFlushConstruct>(
predicated(OmpDirectiveNameParser{},
IsDirective(llvm::omp::Directive::OMPD_flush)) >=
- Parser<OmpDirectiveSpecification>{})))
+ OmpDirectiveSpecificationParser{})))
// 2.14.2 Cancellation Point construct
TYPE_PARSER(sourced( //
construct<OpenMPCancellationPointConstruct>(
predicated(OmpDirectiveNameParser{},
IsDirective(llvm::omp::Directive::OMPD_cancellation_point)) >=
- Parser<OmpDirectiveSpecification>{})))
+ OmpDirectiveSpecificationParser{})))
// 2.14.1 Cancel construct
TYPE_PARSER(sourced( //
construct<OpenMPCancelConstruct>(
predicated(OmpDirectiveNameParser{},
IsDirective(llvm::omp::Directive::OMPD_cancel)) >=
- Parser<OmpDirectiveSpecification>{})))
+ OmpDirectiveSpecificationParser{})))
TYPE_PARSER(sourced( //
construct<OpenMPDepobjConstruct>(
predicated(OmpDirectiveNameParser{},
IsDirective(llvm::omp::Directive::OMPD_depobj)) >=
- Parser<OmpDirectiveSpecification>{})))
+ OmpDirectiveSpecificationParser{})))
// OMP 5.2 14.1 Interop construct
TYPE_PARSER(sourced( //
construct<OpenMPInteropConstruct>(
predicated(OmpDirectiveNameParser{},
IsDirective(llvm::omp::Directive::OMPD_interop)) >=
- Parser<OmpDirectiveSpecification>{})))
+ OmpDirectiveSpecificationParser{})))
// Standalone Constructs
TYPE_PARSER(
@@ -2248,19 +2309,19 @@ TYPE_PARSER(construct<OmpInitializerClause>(Parser<OmpInitializerExpression>{}))
TYPE_PARSER(sourced(construct<OmpDeclareVariantDirective>(
predicated(Parser<OmpDirectiveName>{},
IsDirective(llvm::omp::Directive::OMPD_declare_variant)) >=
- Parser<OmpDirectiveSpecification>{})))
+ OmpDirectiveSpecificationParser{})))
// 2.16 Declare Reduction Construct
TYPE_PARSER(sourced(construct<OpenMPDeclareReductionConstruct>(
predicated(Parser<OmpDirectiveName>{},
IsDirective(llvm::omp::Directive::OMPD_declare_reduction)) >=
- OmpStylizedInstanceCreator(Parser<OmpDirectiveSpecification>{}))))
+ OmpStylizedInstanceCreator(OmpDirectiveSpecificationParser{}))))
// 2.10.6 Declare Target Construct
TYPE_PARSER(sourced(construct<OpenMPDeclareTargetConstruct>(
predicated(Parser<OmpDirectiveName>{},
IsDirective(llvm::omp::Directive::OMPD_declare_target)) >=
- Parser<OmpDirectiveSpecification>{})))
+ OmpDirectiveSpecificationParser{})))
static OmpMapperSpecifier ConstructOmpMapperSpecifier(
std::optional<Name> &&mapperName, TypeSpec &&typeSpec, Name &&varName) {
@@ -2290,7 +2351,7 @@ TYPE_PARSER(applyFunction<OmpMapperSpecifier>(ConstructOmpMapperSpecifier,
TYPE_PARSER(sourced(construct<OpenMPDeclareMapperConstruct>(
predicated(Parser<OmpDirectiveName>{},
IsDirective(llvm::omp::Directive::OMPD_declare_mapper)) >=
- Parser<OmpDirectiveSpecification>{})))
+ OmpDirectiveSpecificationParser{})))
TYPE_PARSER(construct<OmpCombinerExpression>(OmpStylizedExpressionParser{}))
TYPE_PARSER(construct<OmpInitializerExpression>(OmpStylizedExpressionParser{}))
@@ -2302,32 +2363,32 @@ TYPE_PARSER(sourced(construct<OpenMPCriticalConstruct>(
TYPE_PARSER(sourced(construct<OpenMPDeclareSimdConstruct>(
predicated(Parser<OmpDirectiveName>{},
IsDirective(llvm::omp::Directive::OMPD_declare_simd)) >=
- Parser<OmpDirectiveSpecification>{})))
+ OmpDirectiveSpecificationParser{})))
TYPE_PARSER(sourced( //
construct<OpenMPGroupprivate>(
predicated(OmpDirectiveNameParser{},
IsDirective(llvm::omp::Directive::OMPD_groupprivate)) >=
- Parser<OmpDirectiveSpecification>{})))
+ OmpDirectiveSpecificationParser{})))
// 2.4 Requires construct
TYPE_PARSER(sourced(construct<OpenMPRequiresConstruct>(
predicated(OmpDirectiveNameParser{},
IsDirective(llvm::omp::Directive::OMPD_requires)) >=
- Parser<OmpDirectiveSpecification>{})))
+ OmpDirectiveSpecificationParser{})))
// 2.15.2 Threadprivate directive
TYPE_PARSER(sourced( //
construct<OpenMPThreadprivate>(
predicated(OmpDirectiveNameParser{},
IsDirective(llvm::omp::Directive::OMPD_threadprivate)) >=
- Parser<OmpDirectiveSpecification>{})))
+ OmpDirectiveSpecificationParser{})))
// Assumes Construct
TYPE_PARSER(sourced(construct<OpenMPDeclarativeAssumes>(
predicated(OmpDirectiveNameParser{},
IsDirective(llvm::omp::Directive::OMPD_assumes)) >=
- Parser<OmpDirectiveSpecification>{})))
+ OmpDirectiveSpecificationParser{})))
// Declarative constructs
TYPE_PARSER(
@@ -2409,7 +2470,7 @@ TYPE_PARSER(construct<OmpEndSectionsDirective>(
static constexpr auto sectionDir{
startOmpLine >> (predicated(OmpDirectiveNameParser{},
IsDirective(llvm::omp::Directive::OMPD_section)) >=
- Parser<OmpDirectiveSpecification>{})};
+ OmpDirectiveSpecificationParser{})};
// OMP SECTIONS (OpenMP 5.0 - 2.8.1), PARALLEL SECTIONS (OpenMP 5.0 - 2.13.3)
TYPE_PARSER(sourced(construct<OpenMPSectionsConstruct>(
diff --git a/flang/test/Parser/OpenMP/no-commas-in-ods-list-item.f90 b/flang/test/Parser/OpenMP/no-commas-in-ods-list-item.f90
new file mode 100644
index 0000000000000..829ae742cd92f
--- /dev/null
+++ b/flang/test/Parser/OpenMP/no-commas-in-ods-list-item.f90
@@ -0,0 +1,16 @@
+!RUN: not %flang_fc1 -fdebug-unparse -fopenmp -fopenmp-version=60 %s 2>&1 | FileCheck %s
+
+!The "11:" below is a line number. It is the line with the "otherwise" clause.
+!CHECK-LABEL: Could not parse
+!CHECK-LABEL: 11:{{[0-9]+}}: error:
+
+subroutine f
+ integer :: i, j
+
+ !$omp metadirective &
+ !$omp otherwise(parallel do num_threads(2), collapse(2))
+ do i = 1, 10
+ do j = 1, 10
+ end do
+ end do
+end
``````````
</details>
https://github.com/llvm/llvm-project/pull/185737
More information about the flang-commits
mailing list