https://github.com/kparzysz created https://github.com/llvm/llvm-project/pull/168885
Add two more AST nodes, one for a misplaced end-directive, and one for an invalid string following the OpenMP sentinel (e.g. "!$OMP XYZ"). Emit error messages when either node is encountered in semantic analysis. >From 72c046f755c26e7696ccff0ba4e25701293be726 Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek <[email protected]> Date: Wed, 19 Nov 2025 14:41:07 -0600 Subject: [PATCH] [flang][OpenMP] Better diagnostics for invalid or misplaced directives Add two more AST nodes, one for a misplaced end-directive, and one for an invalid string following the OpenMP sentinel (e.g. "!$OMP XYZ"). Emit error messages when either node is encountered in semantic analysis. --- flang/include/flang/Parser/dump-parse-tree.h | 2 + flang/include/flang/Parser/parse-tree.h | 20 +++++++- flang/lib/Parser/executable-parsers.cpp | 2 + flang/lib/Parser/openmp-parsers.cpp | 51 ++++++++++++------- flang/lib/Parser/program-parsers.cpp | 3 ++ flang/lib/Parser/type-parsers.h | 3 +- flang/lib/Parser/unparse.cpp | 10 ++++ flang/lib/Semantics/check-omp-structure.cpp | 23 +++++++++ flang/lib/Semantics/check-omp-structure.h | 6 +++ flang/lib/Semantics/resolve-directives.cpp | 3 ++ flang/test/Parser/OpenMP/fail-construct2.f90 | 2 +- flang/test/Parser/OpenMP/tile-fail.f90 | 4 +- .../Semantics/OpenMP/loop-association.f90 | 14 +++++ 13 files changed, 119 insertions(+), 24 deletions(-) diff --git a/flang/include/flang/Parser/dump-parse-tree.h b/flang/include/flang/Parser/dump-parse-tree.h index 5ca9deeb9b7f6..32fcd4182bed7 100644 --- a/flang/include/flang/Parser/dump-parse-tree.h +++ b/flang/include/flang/Parser/dump-parse-tree.h @@ -755,7 +755,9 @@ class ParseTreeDumper { NODE(parser, OpenMPDispatchConstruct) NODE(parser, OpenMPFlushConstruct) NODE(parser, OpenMPGroupprivate) + NODE(parser, OpenMPInvalidDirective) NODE(parser, OpenMPLoopConstruct) + NODE(parser, OpenMPMisplacedEndDirective) NODE(parser, OpenMPRequiresConstruct) NODE(parser, OpenMPSectionConstruct) NODE(parser, OpenMPSectionsConstruct) diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h index 9795a0d2ae25e..003d11721908e 100644 --- a/flang/include/flang/Parser/parse-tree.h +++ b/flang/include/flang/Parser/parse-tree.h @@ -269,8 +269,9 @@ struct AccEndCombinedDirective; struct OpenACCDeclarativeConstruct; struct OpenACCRoutineConstruct; struct OpenMPConstruct; -struct OpenMPLoopConstruct; struct OpenMPDeclarativeConstruct; +struct OpenMPInvalidDirective; +struct OpenMPMisplacedEndDirective; struct CUFKernelDoConstruct; // Cooked character stream locations @@ -406,6 +407,8 @@ struct SpecificationConstruct { common::Indirection<StructureDef>, common::Indirection<OpenACCDeclarativeConstruct>, common::Indirection<OpenMPDeclarativeConstruct>, + common::Indirection<OpenMPMisplacedEndDirective>, + common::Indirection<OpenMPInvalidDirective>, common::Indirection<CompilerDirective>> u; }; @@ -538,6 +541,8 @@ struct ExecutableConstruct { common::Indirection<OpenACCConstruct>, common::Indirection<AccEndCombinedDirective>, common::Indirection<OpenMPConstruct>, + common::Indirection<OpenMPMisplacedEndDirective>, + common::Indirection<OpenMPInvalidDirective>, common::Indirection<CUFKernelDoConstruct>> u; }; @@ -5379,6 +5384,19 @@ struct OpenMPConstruct { u; }; +// Orphaned !$OMP END <directive>, i.e. not being a part of a valid OpenMP +// construct. +struct OpenMPMisplacedEndDirective : public OmpEndDirective { + INHERITED_TUPLE_CLASS_BOILERPLATE( + OpenMPMisplacedEndDirective, OmpEndDirective); +}; + +// Unrecognized string after the !$OMP sentinel. +struct OpenMPInvalidDirective { + using EmptyTrait = std::true_type; + CharBlock source; +}; + // Parse tree nodes for OpenACC 3.3 directives and clauses struct AccObject { diff --git a/flang/lib/Parser/executable-parsers.cpp b/flang/lib/Parser/executable-parsers.cpp index 8d777a6671495..2241c04f5d26d 100644 --- a/flang/lib/Parser/executable-parsers.cpp +++ b/flang/lib/Parser/executable-parsers.cpp @@ -50,6 +50,8 @@ constexpr auto executableConstruct{first( construct<ExecutableConstruct>(indirect(whereConstruct)), construct<ExecutableConstruct>(indirect(forallConstruct)), construct<ExecutableConstruct>(indirect(openmpConstruct)), + construct<ExecutableConstruct>(indirect(openmpMisplacedEndDirective)), + construct<ExecutableConstruct>(indirect(openmpInvalidDirective)), construct<ExecutableConstruct>(indirect(Parser<OpenACCConstruct>{})), construct<ExecutableConstruct>(indirect(compilerDirective)), construct<ExecutableConstruct>(indirect(Parser<CUFKernelDoConstruct>{})))}; diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp index d50f45794230b..6592b87f8c62d 100644 --- a/flang/lib/Parser/openmp-parsers.cpp +++ b/flang/lib/Parser/openmp-parsers.cpp @@ -1573,6 +1573,14 @@ static inline constexpr auto IsMemberOf(const DirectiveSet &dirs) { }; } +constexpr auto validEPC{// + predicated(executionPartConstruct, [](auto &epc) { + return !Unwrap<OpenMPMisplacedEndDirective>(epc) && + !Unwrap<OpenMPMisplacedEndDirective>(epc); + })}; + +constexpr auto validBlock{many(validEPC)}; + TYPE_PARSER(sourced(construct<OmpDirectiveName>(OmpDirectiveNameParser{}))) OmpDirectiveSpecification static makeFlushFromOldSyntax(Verbatim &&text, @@ -1630,7 +1638,7 @@ struct StrictlyStructuredBlockParser { std::optional<resultType> Parse(ParseState &state) const { // Detect BLOCK construct without parsing the entire thing. if (lookAhead(skipStuffBeforeStatement >> "BLOCK"_tok).Parse(state)) { - if (auto epc{Parser<ExecutionPartConstruct>{}.Parse(state)}) { + if (auto &&epc{executionPartConstruct.Parse(state)}) { if (GetFortranBlockConstruct(*epc) != nullptr) { Block body; body.emplace_back(std::move(*epc)); @@ -1650,7 +1658,7 @@ struct LooselyStructuredBlockParser { if (lookAhead(skipStuffBeforeStatement >> "BLOCK"_tok).Parse(state)) { return std::nullopt; } - if (auto &&body{block.Parse(state)}) { + if (auto &&body{validBlock.Parse(state)}) { // Empty body is ok. return std::move(body); } @@ -1674,7 +1682,7 @@ struct NonBlockDoConstructParser { // Keep parsing ExecutionPartConstructs until the set of open label-do // statements becomes empty, or until the EPC parser fails. - while (auto &&epc{attempt(executionPartConstruct).Parse(state)}) { + while (auto &&epc{attempt(validEPC).Parse(state)}) { if (auto &&label{GetStatementLabel(*epc)}) { labels.erase(*label); } @@ -1825,7 +1833,7 @@ struct OmpStatementConstructParser { std::optional<resultType> Parse(ParseState &state) const { if (auto begin{OmpBeginDirectiveParser(dir_).Parse(state)}) { Block body; - if (auto stmt{attempt(Parser<ExecutionPartConstruct>{}).Parse(state)}) { + if (auto stmt{attempt(validEPC).Parse(state)}) { body.emplace_back(std::move(*stmt)); } // Allow empty block. Check for this in semantics. @@ -1995,11 +2003,9 @@ struct OmpAtomicConstructParser { return std::nullopt; } - auto exec{Parser<ExecutionPartConstruct>{}}; - auto end{OmpEndDirectiveParser{llvm::omp::Directive::OMPD_atomic}}; TailType tail; - if (ParseOne(exec, end, tail, state)) { + if (ParseOne(tail, state)) { if (!tail.first.empty()) { if (auto &&rest{attempt(LimitedTailParser(BodyLimit)).Parse(state)}) { for (auto &&s : rest->first) { @@ -2026,13 +2032,12 @@ struct OmpAtomicConstructParser { // Parse either an ExecutionPartConstruct, or atomic end-directive. When // successful, record the result in the "tail" provided, otherwise fail. - static std::optional<Success> ParseOne( // - Parser<ExecutionPartConstruct> &exec, OmpEndDirectiveParser &end, - TailType &tail, ParseState &state) { - auto isRecovery{[](const ExecutionPartConstruct &e) { - return std::holds_alternative<ErrorRecovery>(e.u); + static std::optional<Success> ParseOne(TailType &tail, ParseState &state) { + auto isUsable{[](const std::optional<ExecutionPartConstruct> &e) { + return e && !std::holds_alternative<ErrorRecovery>(e->u); }}; - if (auto &&stmt{attempt(exec).Parse(state)}; stmt && !isRecovery(*stmt)) { + auto end{OmpEndDirectiveParser{llvm::omp::Directive::OMPD_atomic}}; + if (auto &&stmt{attempt(validEPC).Parse(state)}; isUsable(stmt)) { tail.first.emplace_back(std::move(*stmt)); } else if (auto &&dir{attempt(end).Parse(state)}) { tail.second = std::move(*dir); @@ -2048,12 +2053,10 @@ struct OmpAtomicConstructParser { constexpr LimitedTailParser(size_t count) : count_(count) {} std::optional<resultType> Parse(ParseState &state) const { - auto exec{Parser<ExecutionPartConstruct>{}}; - auto end{OmpEndDirectiveParser{llvm::omp::Directive::OMPD_atomic}}; TailType tail; for (size_t i{0}; i != count_; ++i) { - if (ParseOne(exec, end, tail, state)) { + if (ParseOne(tail, state)) { if (tail.second) { // Return when the end-directive was parsed. return std::move(tail); @@ -2325,9 +2328,9 @@ TYPE_PARSER(sourced(construct<OpenMPSectionsConstruct>( Parser<OmpBeginSectionsDirective>{} / endOmpLine, cons( // construct<OpenMPConstruct>(sourced( - construct<OpenMPSectionConstruct>(maybe(sectionDir), block))), - many(construct<OpenMPConstruct>( - sourced(construct<OpenMPSectionConstruct>(sectionDir, block))))), + construct<OpenMPSectionConstruct>(maybe(sectionDir), validBlock))), + many(construct<OpenMPConstruct>(sourced( + construct<OpenMPSectionConstruct>(sectionDir, validBlock))))), maybe(Parser<OmpEndSectionsDirective>{} / endOmpLine)))) static bool IsExecutionPart(const OmpDirectiveName &name) { @@ -2402,4 +2405,14 @@ static constexpr DirectiveSet GetLoopDirectives() { TYPE_PARSER(sourced(construct<OpenMPLoopConstruct>( OmpLoopConstructParser(GetLoopDirectives())))) +static constexpr DirectiveSet GetAllDirectives() { // + return ~DirectiveSet{}; +} + +TYPE_PARSER(construct<OpenMPMisplacedEndDirective>( + OmpEndDirectiveParser{GetAllDirectives()})) + +TYPE_PARSER( // + startOmpLine >> sourced(construct<OpenMPInvalidDirective>( + !OmpDirectiveNameParser{} >> SkipTo<'\n'>{}))) } // namespace Fortran::parser diff --git a/flang/lib/Parser/program-parsers.cpp b/flang/lib/Parser/program-parsers.cpp index 740dbbfab9db7..303335934a37a 100644 --- a/flang/lib/Parser/program-parsers.cpp +++ b/flang/lib/Parser/program-parsers.cpp @@ -201,6 +201,9 @@ TYPE_CONTEXT_PARSER("specification construct"_en_US, construct<SpecificationConstruct>( indirect(openaccDeclarativeConstruct)), construct<SpecificationConstruct>(indirect(openmpDeclarativeConstruct)), + construct<SpecificationConstruct>( + indirect(openmpMisplacedEndDirective)), + construct<SpecificationConstruct>(indirect(openmpInvalidDirective)), construct<SpecificationConstruct>(indirect(compilerDirective)))) // R513 other-specification-stmt -> diff --git a/flang/lib/Parser/type-parsers.h b/flang/lib/Parser/type-parsers.h index 3900c5a86c874..142aa226893b6 100644 --- a/flang/lib/Parser/type-parsers.h +++ b/flang/lib/Parser/type-parsers.h @@ -139,7 +139,8 @@ constexpr Parser<OpenACCDeclarativeConstruct> openaccDeclarativeConstruct; constexpr Parser<OpenMPConstruct> openmpConstruct; constexpr Parser<OpenMPExecDirective> openmpExecDirective; constexpr Parser<OpenMPDeclarativeConstruct> openmpDeclarativeConstruct; -constexpr Parser<OmpEndLoopDirective> ompEndLoopDirective; +constexpr Parser<OpenMPMisplacedEndDirective> openmpMisplacedEndDirective; +constexpr Parser<OpenMPInvalidDirective> openmpInvalidDirective; constexpr Parser<IntrinsicVectorTypeSpec> intrinsicVectorTypeSpec; // Extension constexpr Parser<VectorTypeSpec> vectorTypeSpec; // Extension constexpr Parser<UnsignedTypeSpec> unsignedTypeSpec; // Extension diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp index f81200d092b11..3854d33d46d48 100644 --- a/flang/lib/Parser/unparse.cpp +++ b/flang/lib/Parser/unparse.cpp @@ -2706,6 +2706,16 @@ class UnparseVisitor { Put("\n"); EndOpenMP(); } + void Unparse(const OpenMPMisplacedEndDirective &x) { + Unparse(static_cast<const OmpEndDirective &>(x)); + } + void Unparse(const OpenMPInvalidDirective &x) { + BeginOpenMP(); + Word("!$OMP "); + Put(parser::ToUpperCaseLetters(x.source.ToString())); + Put("\n"); + EndOpenMP(); + } void Unparse(const BasedPointer &x) { Put('('), Walk(std::get<0>(x.t)), Put(","), Walk(std::get<1>(x.t)); Walk("(", std::get<std::optional<ArraySpec>>(x.t), ")"), Put(')'); diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp index 37b4404cc598f..6c18b7a8ac4bd 100644 --- a/flang/lib/Semantics/check-omp-structure.cpp +++ b/flang/lib/Semantics/check-omp-structure.cpp @@ -5474,6 +5474,29 @@ void OmpStructureChecker::CheckAllowedRequiresClause(llvmOmpClause clause) { } } +void OmpStructureChecker::Enter(const parser::OpenMPMisplacedEndDirective &x) { + invalidState_ = true; + context_.Say(x.DirName().source, "Misplaced OpenMP end-directive"_err_en_US); + PushContextAndClauseSets( + x.DirName().source, llvm::omp::Directive::OMPD_unknown); +} + +void OmpStructureChecker::Leave(const parser::OpenMPMisplacedEndDirective &x) { + dirContext_.pop_back(); + invalidState_ = false; +} + +void OmpStructureChecker::Enter(const parser::OpenMPInvalidDirective &x) { + invalidState_ = true; + context_.Say(x.source, "Invalid OpenMP directive"_err_en_US); + PushContextAndClauseSets(x.source, llvm::omp::Directive::OMPD_unknown); +} + +void OmpStructureChecker::Leave(const parser::OpenMPInvalidDirective &x) { + dirContext_.pop_back(); + invalidState_ = false; +} + // Use when clause falls under 'struct OmpClause' in 'parse-tree.h'. #define CHECK_SIMPLE_CLAUSE(X, Y) \ void OmpStructureChecker::Enter(const parser::OmpClause::X &) { \ diff --git a/flang/lib/Semantics/check-omp-structure.h b/flang/lib/Semantics/check-omp-structure.h index 7139f475e91d6..965f18bfbb352 100644 --- a/flang/lib/Semantics/check-omp-structure.h +++ b/flang/lib/Semantics/check-omp-structure.h @@ -94,6 +94,11 @@ class OmpStructureChecker : public OmpStructureCheckerBase { void Enter(const parser::OpenMPDeclarativeConstruct &); void Leave(const parser::OpenMPDeclarativeConstruct &); + void Enter(const parser::OpenMPMisplacedEndDirective &); + void Leave(const parser::OpenMPMisplacedEndDirective &); + void Enter(const parser::OpenMPInvalidDirective &); + void Leave(const parser::OpenMPInvalidDirective &); + void Enter(const parser::OpenMPLoopConstruct &); void Leave(const parser::OpenMPLoopConstruct &); void Enter(const parser::OmpEndLoopDirective &); @@ -389,6 +394,7 @@ class OmpStructureChecker : public OmpStructureCheckerBase { std::vector<LoopConstruct> loopStack_; // Scopes for scoping units. std::vector<const Scope *> scopeStack_; + bool invalidState_{false}; // Set during visiting OpenMPMisplacedEndDirective enum class PartKind : int { // There are also other "parts", such as internal-subprogram-part, etc, diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp index f69fce8a6b17a..4fadcf73c5cbb 100644 --- a/flang/lib/Semantics/resolve-directives.cpp +++ b/flang/lib/Semantics/resolve-directives.cpp @@ -529,6 +529,9 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor<llvm::omp::Directive> { void Post(const parser::OmpBeginLoopDirective &) { GetContext().withinConstruct = true; } + bool Pre(const parser::OpenMPMisplacedEndDirective &x) { return false; } + bool Pre(const parser::OpenMPInvalidDirective &x) { return false; } + bool Pre(const parser::DoConstruct &); bool Pre(const parser::OpenMPSectionsConstruct &); diff --git a/flang/test/Parser/OpenMP/fail-construct2.f90 b/flang/test/Parser/OpenMP/fail-construct2.f90 index b7f5736d1329b..3798c3dae3f0d 100644 --- a/flang/test/Parser/OpenMP/fail-construct2.f90 +++ b/flang/test/Parser/OpenMP/fail-construct2.f90 @@ -1,5 +1,5 @@ ! RUN: not %flang_fc1 -fsyntax-only -fopenmp %s 2>&1 | FileCheck %s -! CHECK: error: expected OpenMP construct +! CHECK: error: Invalid OpenMP directive !$omp dummy end diff --git a/flang/test/Parser/OpenMP/tile-fail.f90 b/flang/test/Parser/OpenMP/tile-fail.f90 index 3cb0ea96975c8..a69261a927961 100644 --- a/flang/test/Parser/OpenMP/tile-fail.f90 +++ b/flang/test/Parser/OpenMP/tile-fail.f90 @@ -8,7 +8,7 @@ ! Parser error subroutine stray_end1 - !CHECK: error: expected OpenMP construct + !CHECK: error: Misplaced OpenMP end-directive !$omp end tile end subroutine @@ -17,7 +17,7 @@ subroutine stray_end1 subroutine stray_end2 print * - !CHECK: error: expected 'END' + !CHECK: error: Misplaced OpenMP end-directive !$omp end tile end subroutine diff --git a/flang/test/Semantics/OpenMP/loop-association.f90 b/flang/test/Semantics/OpenMP/loop-association.f90 index 0a3462048000e..603bfdcc0a4e9 100644 --- a/flang/test/Semantics/OpenMP/loop-association.f90 +++ b/flang/test/Semantics/OpenMP/loop-association.f90 @@ -81,6 +81,8 @@ do i = 1, N enddo !$omp end parallel do + !ERROR: Misplaced OpenMP end-directive + !$omp end parallel do !$omp parallel a = 3.0 @@ -96,6 +98,8 @@ !$omp end parallel a = 0.0 + !ERROR: Misplaced OpenMP end-directive + !$omp end parallel do !$omp parallel do private(c) do i = 1, N do j = 1, N @@ -103,8 +107,12 @@ !ERROR: OpenMP loop construct can only contain DO loops or loop-nest-generating OpenMP constructs a = 3.14 enddo + !ERROR: Misplaced OpenMP end-directive + !$omp end parallel do enddo a = 1.414 + !ERROR: Misplaced OpenMP end-directive + !$omp end parallel do do i = 1, N !$omp parallel do @@ -112,6 +120,8 @@ a = 3.14 enddo enddo + !ERROR: Misplaced OpenMP end-directive + !$omp end parallel do !$omp parallel do private(c) !ERROR: OpenMP loop construct can only contain DO loops or loop-nest-generating OpenMP constructs @@ -119,12 +129,16 @@ do i=1, N a = 3.14 enddo + !ERROR: Misplaced OpenMP end-directive + !$omp end parallel do !$omp parallel do simd do i = 1, N a = 3.14 enddo !$omp end parallel do simd + !ERROR: Misplaced OpenMP end-directive + !$omp end parallel do simd !$omp simd !ERROR: OpenMP loop construct can only contain DO loops or loop-nest-generating OpenMP constructs _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
