[flang] Improve syntax error messages by fixing withMessage() parser combinator

The parser combinator withMessage("error message"_err_en_US, PARSER) is meant
to run the parser PARSER and, if it fails, override its error messages if
it failed silently or it was unable to recognize any tokens at all.  This
gives the parser a way to avoid emitting some confusing or missing error
messages.  Unfortunately, the implementation could sometimes lose track of
whether any tokens had been recognized, leading to problems with outer usage
of withMessage() and also -- more seriously -- with ParseState::CombineFailedParses().
That's a utility that determines which error messages to retain when two
or more parsers have been attempted at the same starting point and none
of them succceed.  Its policy is to retain the state from the parser that
consumed the most input text before failing, so long as it had recognized at
least one token.

So anyway, fix up withMessage(), adjust the tests, and add a test of the
original motivating confusing error situation, in which a syntax error in
a COMMON statement was being diagnosed as a problem with a statement function
definition because withMessage() had lost the fact that the parse of the
COMMON statement had recognized some tokens, and the last attempted parse
later was a failed attempt to parse a statement function.

Differential Revision: https://reviews.llvm.org/D135216

flang/lib/Parser/basic-parsers.h

@@ -208,26 +208,31 @@ public:
   constexpr WithMessageParser(MessageFixedText t, PA p)
       : text_{t}, parser_{p} {}
   std::optional<resultType> Parse(ParseState &state) const {
+    if (state.deferMessages()) { // fast path
+      std::optional<resultType> result{parser_.Parse(state)};
+      if (!result) {
+        state.set_anyDeferredMessages();
+      }
+      return result;
+    }
     Messages messages{std::move(state.messages())};
-    ParseState backtrack{state};
+    bool hadAnyTokenMatched{state.anyTokenMatched()};
     state.set_anyTokenMatched(false);
     std::optional<resultType> result{parser_.Parse(state)};
     bool emitMessage{false};
     if (result) {
       messages.Annex(std::move(state.messages()));
-      if (backtrack.anyTokenMatched()) {
+      if (hadAnyTokenMatched) {
         state.set_anyTokenMatched();
       }
     } else if (state.anyTokenMatched()) {
       emitMessage = state.messages().empty();
       messages.Annex(std::move(state.messages()));
-      backtrack.set_anyTokenMatched();
-      if (state.anyDeferredMessages()) {
-        backtrack.set_anyDeferredMessages(true);
-      }
-      state = std::move(backtrack);
     } else {
       emitMessage = true;
+      if (hadAnyTokenMatched) {
+        state.set_anyTokenMatched();
+      }
     }
     state.messages() = std::move(messages);
     if (emitMessage) {
@@ -351,7 +356,7 @@ public:
   using resultType = typename PA::resultType;
   static_assert(std::is_same_v<resultType, typename PB::resultType>);
   constexpr RecoveryParser(const RecoveryParser &) = default;
-  constexpr RecoveryParser(PA pa, PB pb) : pa_{pa}, pb3_{pb} {}
+  constexpr RecoveryParser(PA pa, PB pb) : pa_{pa}, pb_{pb} {}
   std::optional<resultType> Parse(ParseState &state) const {
     bool originallyDeferred{state.deferMessages()};
     ParseState backtrack{state};
@@ -379,7 +384,7 @@ public:
     bool anyTokenMatched{state.anyTokenMatched()};
     state = std::move(backtrack);
     state.set_deferMessages(true);
-    std::optional<resultType> bx{pb3_.Parse(state)};
+    std::optional<resultType> bx{pb_.Parse(state)};
     state.messages() = std::move(messages);
     state.set_deferMessages(originallyDeferred);
     if (anyTokenMatched) {
@@ -398,7 +403,7 @@ public:
 
 private:
   const PA pa_;
-  const PB pb3_;
+  const PB pb_;
 };
 
 template <typename PA, typename PB>

flang/test/Driver/color-diagnostics-parse.f90

@@ -11,9 +11,9 @@
 ! RUN:     | FileCheck %s --check-prefix=CHECK_CD
 ! RUN: not %flang_fc1 %s 2>&1 | FileCheck %s --check-prefix=CHECK_NCD
 
-! CHECK_CD: {{.*}}[0;1;31merror: {{.*}}[0mexpected '('
+! CHECK_CD: {{.*}}[0;1;31merror: {{.*}}[0mexpected end of statement
 
-! CHECK_NCD: error: expected '('
+! CHECK_NCD: error: expected end of statement
 
 program m
   integer :: i =

flang/test/Parser/doubled-comma.f90

@@ -0,0 +1,4 @@
+! RUN: not %flang_fc1 -fsyntax-only %s 2>&1 | FileCheck %s
+! CHECK: 3:13: error: expected end of statement
+common/blk/a,,b
+end

flang/test/Semantics/error_stop1a.f90

@@ -43,47 +43,47 @@ program test_error_stop
   !___ non-standard-conforming statements _________________________
 
   ! unknown stop-code
-  !ERROR: expected execution part construct
+  !ERROR: expected end of statement
   error stop code=int_code
 
   ! missing 'quiet='
-  !ERROR: expected execution part construct
+  !ERROR: expected end of statement
   error stop int_code, bool
 
   ! incorrect spelling for 'quiet='
-  !ERROR: expected execution part construct
+  !ERROR: expected end of statement
   error stop int_code, quiets=bool
 
   ! missing scalar-logical-expr for quiet=
-  !ERROR: expected execution part construct
+  !ERROR: expected end of statement
   error stop int_code, quiet
 
   ! superfluous stop-code
-  !ERROR: expected execution part construct
+  !ERROR: expected end of statement
   error stop int_code, char_code
 
   ! repeated quiet=
-  !ERROR: expected execution part construct
+  !ERROR: expected end of statement
   error stop int_code, quiet=bool, quiet=.true.
 
   ! superfluous stop-code
-  !ERROR: expected execution part construct
+  !ERROR: expected end of statement
   error stop int_code, char_code, quiet=bool
 
   ! superfluous integer
-  !ERROR: expected execution part construct
+  !ERROR: expected end of statement
   error stop int_code, quiet=bool, 5
 
   ! quiet= appears without stop-code
-  !ERROR: expected execution part construct
+  !ERROR: expected end of statement
   error stop quiet=bool
 
   ! incorrect syntax
-  !ERROR: expected execution part construct
+  !ERROR: expected end of statement
   error stop ()
 
   ! incorrect syntax
-  !ERROR: expected execution part construct
+  !ERROR: expected end of statement
   error stop (2, quiet=.true.)
 
 end program test_error_stop

flang/test/Semantics/synchronization01a.f90

@@ -20,29 +20,29 @@ program test_sync_all
 
   !______ invalid sync-stat-lists: invalid stat= ____________
 
-  !ERROR: expected execution part construct
+  !ERROR: expected end of statement
   sync all(status=sync_status)
 
   ! Invalid sync-stat-list: missing stat-variable
-  !ERROR: expected execution part construct
+  !ERROR: expected end of statement
   sync all(stat)
 
   ! Invalid sync-stat-list: missing 'stat='
-  !ERROR: expected execution part construct
+  !ERROR: expected end of statement
   sync all(sync_status)
 
   !______ invalid sync-stat-lists: invalid errmsg= ____________
 
   ! Invalid errmsg-variable keyword
-  !ERROR: expected execution part construct
+  !ERROR: expected end of statement
   sync all(errormsg=error_message)
 
   ! Invalid sync-stat-list: missing 'errmsg='
-  !ERROR: expected execution part construct
+  !ERROR: expected end of statement
   sync all(error_message)
 
   ! Invalid sync-stat-list: missing errmsg-variable
-  !ERROR: expected execution part construct
+  !ERROR: expected end of statement
   sync all(errmsg)
 
 end program test_sync_all

flang/test/Semantics/synchronization03a.f90

@@ -20,29 +20,29 @@ program test_sync_memory
 
   !______ invalid sync-stat-lists: invalid stat= ____________
 
-  !ERROR: expected execution part construct
+  !ERROR: expected end of statement
   sync memory(status=sync_status)
 
   ! Invalid sync-stat-list: missing stat-variable
-  !ERROR: expected execution part construct
+  !ERROR: expected end of statement
   sync memory(stat)
 
   ! Invalid sync-stat-list: missing 'stat='
-  !ERROR: expected execution part construct
+  !ERROR: expected end of statement
   sync memory(sync_status)
 
   !______ invalid sync-stat-lists: invalid errmsg= ____________
 
   ! Invalid errmsg-variable keyword
-  !ERROR: expected execution part construct
+  !ERROR: expected end of statement
   sync memory(errormsg=error_message)
 
   ! Invalid sync-stat-list: missing 'errmsg='
-  !ERROR: expected execution part construct
+  !ERROR: expected end of statement
   sync memory(error_message)
 
   ! Invalid sync-stat-list: missing errmsg-variable
-  !ERROR: expected execution part construct
+  !ERROR: expected end of statement
   sync memory(errmsg)
 
 end program test_sync_memory