Fix typo

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).

flang/include/flang/Parser/parse-tree.h

@@ -4893,8 +4893,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 within the
+  // parser. Semantics will generate an error if this is absent.
   std::tuple<OmpBeginSectionsDirective, std::list<OpenMPConstruct>,
-      OmpEndSectionsDirective>
+      std::optional<OmpEndSectionsDirective>>
       t;
 };
 

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 =

flang/lib/Parser/openmp-parsers.cpp

@@ -1484,11 +1484,25 @@ 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)); })};
       }
     }
     return std::nullopt;
@@ -1903,7 +1917,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();

flang/lib/Parser/unparse.cpp

@@ -2788,7 +2788,7 @@ public:
     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();
   }

flang/lib/Semantics/check-omp-structure.cpp

@@ -785,6 +785,30 @@ 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.
+  // The end block is mandatory when the construct is not applied to a strictly
+  // structured block (aka it is applied to a loosely structured block). In
+  // other words, the body doesn't contain exactly one parser::BlockConstruct.
+  auto isStrictlyStructuredBlock{[](const parser::Block &block) -> bool {
+    if (block.size() != 1) {
+      return false;
+    }
+    const parser::ExecutionPartConstruct &contents{block.front()};
+    auto *executableConstruct{
+        std::get_if<parser::ExecutableConstruct>(&contents.u)};
+    if (!executableConstruct) {
+      return false;
+    }
+    return std::holds_alternative<common::Indirection<parser::BlockConstruct>>(
+        executableConstruct->u);
+  }};
+  if (!endSpec && !isStrictlyStructuredBlock(block)) {
+    context_.Say(
+        x.BeginDir().source, "Expected OpenMP end directive"_err_en_US);
+  }
+
   if (llvm::omp::allTargetSet.test(GetContext().directive)) {
     EnterDirectiveNest(TargetNest);
   }
@@ -1039,14 +1063,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 &sectionBlocks{std::get<std::list<parser::OpenMPConstruct>>(x.t)};
   for (const parser::OpenMPConstruct &construct : sectionBlocks) {

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

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

flang/test/Semantics/OpenMP/missing-end-directive.f90

@@ -0,0 +1,17 @@
+! 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 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