llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-flang-semantics Author: Tom Eccles (tblah) <details> <summary>Changes</summary> See #<!-- -->90452. The old parse tree errors exploded to thousands of unhelpful lines when there were multiple missing end directives. Instead, allow a missing end directive in the parse tree then validate that it is present during semantics (where the error messages are a lot easier to control). --- Full diff: https://github.com/llvm/llvm-project/pull/154740.diff 6 Files Affected: - (modified) flang/include/flang/Parser/parse-tree.h (+4-1) - (modified) flang/lib/Lower/OpenMP/OpenMP.cpp (+5-2) - (modified) flang/lib/Parser/openmp-parsers.cpp (+1-1) - (modified) flang/lib/Parser/unparse.cpp (+1-1) - (modified) flang/lib/Semantics/check-omp-structure.cpp (+13-4) - (modified) flang/test/Semantics/OpenMP/missing-end-directive.f90 (+4) ``````````diff diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h index 38ec605574c06..9962a25c29600 100644 --- a/flang/include/flang/Parser/parse-tree.h +++ b/flang/include/flang/Parser/parse-tree.h @@ -4903,8 +4903,11 @@ struct OpenMPSectionsConstruct { CharBlock source; // Each of the OpenMPConstructs in the list below contains an // OpenMPSectionConstruct. This is guaranteed by the parser. + // The end sections directive is optional here because it is difficult to + // generate helpful error messages for a missing end directive wihtin the + // parser. Semantics will generate an error if this is absent. std::tuple<OmpBeginSectionsDirective, std::list<OpenMPConstruct>, - OmpEndSectionsDirective> + std::optional<OmpEndSectionsDirective>> t; }; diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp index ec2ec37e623f8..da5898480da22 100644 --- a/flang/lib/Lower/OpenMP/OpenMP.cpp +++ b/flang/lib/Lower/OpenMP/OpenMP.cpp @@ -3958,9 +3958,12 @@ static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable, List<Clause> clauses = makeClauses( std::get<parser::OmpClauseList>(beginSectionsDirective.t), semaCtx); const auto &endSectionsDirective = - std::get<parser::OmpEndSectionsDirective>(sectionsConstruct.t); + std::get<std::optional<parser::OmpEndSectionsDirective>>( + sectionsConstruct.t); + assert(endSectionsDirective && + "Missing end section directive should have been handled in semantics"); clauses.append(makeClauses( - std::get<parser::OmpClauseList>(endSectionsDirective.t), semaCtx)); + std::get<parser::OmpClauseList>(endSectionsDirective->t), semaCtx)); mlir::Location currentLocation = converter.getCurrentLocation(); llvm::omp::Directive directive = diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp index 9670302c8549b..d7db3d55c3e17 100644 --- a/flang/lib/Parser/openmp-parsers.cpp +++ b/flang/lib/Parser/openmp-parsers.cpp @@ -1919,7 +1919,7 @@ TYPE_PARSER(sourced(construct<OpenMPSectionsConstruct>( construct<OpenMPSectionConstruct>(maybe(sectionDir), block))), many(construct<OpenMPConstruct>( sourced(construct<OpenMPSectionConstruct>(sectionDir, block))))), - Parser<OmpEndSectionsDirective>{} / endOmpLine))) + maybe(Parser<OmpEndSectionsDirective>{} / endOmpLine)))) static bool IsExecutionPart(const OmpDirectiveName &name) { return name.IsExecutionPart(); diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp index 09dcfe60a46bc..87e699dbc4e8d 100644 --- a/flang/lib/Parser/unparse.cpp +++ b/flang/lib/Parser/unparse.cpp @@ -2788,7 +2788,7 @@ class UnparseVisitor { Walk(std::get<std::list<OpenMPConstruct>>(x.t), ""); BeginOpenMP(); Word("!$OMP END "); - Walk(std::get<OmpEndSectionsDirective>(x.t)); + Walk(std::get<std::optional<OmpEndSectionsDirective>>(x.t)); Put("\n"); EndOpenMP(); } diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp index 0bdd2c62f88ce..4a52e06cccb1f 100644 --- a/flang/lib/Semantics/check-omp-structure.cpp +++ b/flang/lib/Semantics/check-omp-structure.cpp @@ -1047,14 +1047,23 @@ void OmpStructureChecker::Leave(const parser::OmpBeginDirective &) { void OmpStructureChecker::Enter(const parser::OpenMPSectionsConstruct &x) { const auto &beginSectionsDir{ std::get<parser::OmpBeginSectionsDirective>(x.t)}; - const auto &endSectionsDir{std::get<parser::OmpEndSectionsDirective>(x.t)}; + const auto &endSectionsDir{ + std::get<std::optional<parser::OmpEndSectionsDirective>>(x.t)}; const auto &beginDir{ std::get<parser::OmpSectionsDirective>(beginSectionsDir.t)}; - const auto &endDir{std::get<parser::OmpSectionsDirective>(endSectionsDir.t)}; + PushContextAndClauseSets(beginDir.source, beginDir.v); + + if (!endSectionsDir) { + context_.Say(beginSectionsDir.source, + "Expected OpenMP END SECTIONS directive"_err_en_US); + // Following code assumes the option is present. + return; + } + + const auto &endDir{std::get<parser::OmpSectionsDirective>(endSectionsDir->t)}; CheckMatching<parser::OmpSectionsDirective>(beginDir, endDir); - PushContextAndClauseSets(beginDir.source, beginDir.v); - AddEndDirectiveClauses(std::get<parser::OmpClauseList>(endSectionsDir.t)); + AddEndDirectiveClauses(std::get<parser::OmpClauseList>(endSectionsDir->t)); const auto §ionBlocks{std::get<std::list<parser::OpenMPConstruct>>(x.t)}; for (const parser::OpenMPConstruct &construct : sectionBlocks) { diff --git a/flang/test/Semantics/OpenMP/missing-end-directive.f90 b/flang/test/Semantics/OpenMP/missing-end-directive.f90 index 3b870d134155b..33481f9d650f4 100644 --- a/flang/test/Semantics/OpenMP/missing-end-directive.f90 +++ b/flang/test/Semantics/OpenMP/missing-end-directive.f90 @@ -6,8 +6,12 @@ !$omp parallel ! ERROR: Expected OpenMP end directive !$omp task +! ERROR: Expected OpenMP END SECTIONS directive +!$omp sections ! ERROR: Expected OpenMP end directive !$omp parallel ! ERROR: Expected OpenMP end directive !$omp task +! ERROR: Expected OpenMP END SECTIONS directive +!$omp sections end `````````` </details> https://github.com/llvm/llvm-project/pull/154740 _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
