[flang-commits] [flang] be6bc6a - [flang] Better recovery from errors in a loop control (#117025)

[via flang-commits]

Author: Peter Klausler
Date: 2024-11-21T10:47:42-08:00
New Revision: be6bc6a1e5beb84984b8e1419393c80a3fe2d3d8

URL: https://github.com/llvm/llvm-project/commit/be6bc6a1e5beb84984b8e1419393c80a3fe2d3d8
DIFF: https://github.com/llvm/llvm-project/commit/be6bc6a1e5beb84984b8e1419393c80a3fe2d3d8.diff

LOG: [flang] Better recovery from errors in a loop control (#117025)

When there's an error in a DO statement loop control, error recovery
isn't great. A bare "DO" is a valid statement, so a failure to parse its
loop control doesn't fail on the whole statement. Its partial parse ends
after the keyword, and as some other statement parsers can get further
into the input before failing, errors in the loop control can lead to
confusing error messages about bad pointer assignment statements and
others. So just check that a bare "DO" is followed by the end of the
statement.

Added: 
    flang/test/Parser/recovery07.f90

Modified: 
    flang/docs/ParserCombinators.md
    flang/lib/Parser/executable-parsers.cpp
    flang/lib/Parser/type-parsers.h

Removed: 
    


################################################################################
diff  --git a/flang/docs/ParserCombinators.md b/flang/docs/ParserCombinators.md
index 2c5652ec36138b..7cb77deba21971 100644
--- a/flang/docs/ParserCombinators.md
+++ b/flang/docs/ParserCombinators.md
@@ -178,3 +178,16 @@ is built.  All of the following parsers consume characters acquired from
 Last, a string literal `"..."_debug` denotes a parser that emits the string to
 `llvm::errs` and succeeds.  It is useful for tracing while debugging a parser but should
 obviously not be committed for production code.
+
+### Messages
+A list of generated error and warning messages is maintained in the `ParseState`.
+The parser combinator that handles alternatives (`||` and `first()`) will
+discard the messages from alternatives that fail when there is an alternative
+that succeeds.
+But when no alternative succeeds, and the alternative parser as a whole is
+failing, the messages that survive are chosen from the alternative that
+recognized any input tokens, if only one alternative did so;
+and when multiple alternatives recognized tokens, the messages from the
+alternative that proceeded the furthest into the input are retained.
+This strategy tends to show the most useful error messages to the user
+in situations where a statement fails to parse.

diff  --git a/flang/lib/Parser/executable-parsers.cpp b/flang/lib/Parser/executable-parsers.cpp
index 730165613d91db..ecabbe1db6def6 100644
--- a/flang/lib/Parser/executable-parsers.cpp
+++ b/flang/lib/Parser/executable-parsers.cpp
@@ -9,7 +9,6 @@
 // Per-type parsers for executable statements
 
 #include "basic-parsers.h"
-#include "debug-parser.h"
 #include "expr-parsers.h"
 #include "misc-parsers.h"
 #include "stmt-parser.h"
@@ -282,18 +281,26 @@ TYPE_CONTEXT_PARSER("loop control"_en_US,
                 "CONCURRENT" >> concurrentHeader,
                 many(Parser<LocalitySpec>{})))))
 
+// "DO" is a valid statement, so the loop control is optional; but for
+// better recovery from errors in the loop control, don't parse a
+// DO statement with a bad loop control as a DO statement that has
+// no loop control and is followed by garbage.
+static constexpr auto loopControlOrEndOfStmt{
+    construct<std::optional<LoopControl>>(Parser<LoopControl>{}) ||
+    lookAhead(";\n"_ch) >> construct<std::optional<LoopControl>>()};
+
 // R1121 label-do-stmt -> [do-construct-name :] DO label [loop-control]
 // A label-do-stmt with a do-construct-name is parsed as a nonlabel-do-stmt
 // with an optional label.
 TYPE_CONTEXT_PARSER("label DO statement"_en_US,
-    construct<LabelDoStmt>("DO" >> label, maybe(loopControl)))
+    construct<LabelDoStmt>("DO" >> label, loopControlOrEndOfStmt))
 
 // R1122 nonlabel-do-stmt -> [do-construct-name :] DO [loop-control]
 TYPE_CONTEXT_PARSER("nonlabel DO statement"_en_US,
     construct<NonLabelDoStmt>(
-        name / ":", "DO" >> maybe(label), maybe(loopControl)) ||
+        name / ":", "DO" >> maybe(label), loopControlOrEndOfStmt) ||
         construct<NonLabelDoStmt>(construct<std::optional<Name>>(),
-            construct<std::optional<Label>>(), "DO" >> maybe(loopControl)))
+            construct<std::optional<Label>>(), "DO" >> loopControlOrEndOfStmt))
 
 // R1132 end-do-stmt -> END DO [do-construct-name]
 TYPE_CONTEXT_PARSER("END DO statement"_en_US,

diff  --git a/flang/lib/Parser/type-parsers.h b/flang/lib/Parser/type-parsers.h
index adbf6d23cbd99a..f800398a22de9a 100644
--- a/flang/lib/Parser/type-parsers.h
+++ b/flang/lib/Parser/type-parsers.h
@@ -102,7 +102,6 @@ constexpr Parser<ForallAssignmentStmt> forallAssignmentStmt; // R1053
 constexpr Parser<ForallStmt> forallStmt; // R1055
 constexpr Parser<Selector> selector; // R1105
 constexpr Parser<EndSelectStmt> endSelectStmt; // R1143 & R1151 & R1155
-constexpr Parser<LoopControl> loopControl; // R1123
 constexpr Parser<ConcurrentHeader> concurrentHeader; // R1125
 constexpr Parser<IoUnit> ioUnit; // R1201, R1203
 constexpr Parser<FileUnitNumber> fileUnitNumber; // R1202

diff  --git a/flang/test/Parser/recovery07.f90 b/flang/test/Parser/recovery07.f90
new file mode 100644
index 00000000000000..3a66779b595819
--- /dev/null
+++ b/flang/test/Parser/recovery07.f90
@@ -0,0 +1,6 @@
+! RUN: not %flang_fc1 -fsyntax-only %s 2>&1 | FileCheck %s
+! CHECK: error: expected ':'
+! CHECK: in the context: loop control
+do concurrent(I = 1, N)
+end do
+end