[flang-commits] [flang] [flang] Improve error recovery in tricky situation (PR #95168)
Peter Klausler via flang-commits
flang-commits at lists.llvm.org
Tue Jun 11 13:27:10 PDT 2024
https://github.com/klausler created https://github.com/llvm/llvm-project/pull/95168
When the very first statement of the executable part has syntax errors, it's not at all obvious whether the error messages that are reported to the user should be those from its failure to be the last statement of the specification part or its failure to be the first executable statement when both failures are at the same character in the cooked character stream. Fortran makes this problem more exciting by allowing statement function definitions look a lot like several executable statements.
The current error recovery scheme for declaration constructs depends on a look-ahead test to see whether the failed construct is actually the first executable statement. This works fine when the first executable statement is not in error, but should also allow for some error cases that begin with the tokens of an executable statement.
This can obviously still go wrong for declaration constructs that are unparseable and also have ambiguity in their leading tokens with executable statements, but that seems to be a less likely case.
Also improves error recovery for parenthesized items.
>From 4c95b2162d4f2cf6eaa0eab76a0e4dad0d0ba1aa Mon Sep 17 00:00:00 2001
From: Peter Klausler <pklausler at nvidia.com>
Date: Tue, 11 Jun 2024 10:04:55 -0700
Subject: [PATCH] [flang] Improve error recovery in tricky situation
When the very first statement of the executable part has syntax
errors, it's not at all obvious whether the error messages that
are reported to the user should be those from its failure to be
the last statement of the specification part or its failure to
be the first executable statement when both failures are at the
same character in the cooked character stream. Fortran makes
this problem more exciting by allowing statement function definitions
look a lot like several executable statements.
The current error recovery scheme for declaration constructs
depends on a look-ahead test to see whether the failed construct
is actually the first executable statement. This works fine
when the first executable statement is not in error, but should
also allow for some error cases that begin with the tokens of an
executable statement.
This can obviously still go wrong for declaration constructs that
are unparseable and also have ambiguity in their leading tokens with
executable statements, but that seems to be a less likely case.
Also improves error recovery for parenthesized items.
---
flang/lib/Parser/expr-parsers.cpp | 3 ++-
flang/lib/Parser/program-parsers.cpp | 24 ++++++++++++++++--------
flang/lib/Parser/token-parsers.h | 7 ++++++-
flang/test/Parser/recovery01.f90 | 10 ++++++++++
flang/test/Parser/recovery02.f90 | 8 ++++++++
5 files changed, 42 insertions(+), 10 deletions(-)
create mode 100644 flang/test/Parser/recovery01.f90
create mode 100644 flang/test/Parser/recovery02.f90
diff --git a/flang/lib/Parser/expr-parsers.cpp b/flang/lib/Parser/expr-parsers.cpp
index a47aae166b575..0eaa2dd4ba703 100644
--- a/flang/lib/Parser/expr-parsers.cpp
+++ b/flang/lib/Parser/expr-parsers.cpp
@@ -70,7 +70,8 @@ TYPE_PARSER(construct<AcImpliedDoControl>(
constexpr auto primary{instrumented("primary"_en_US,
first(construct<Expr>(indirect(Parser<CharLiteralConstantSubstring>{})),
construct<Expr>(literalConstant),
- construct<Expr>(construct<Expr::Parentheses>(parenthesized(expr))),
+ construct<Expr>(construct<Expr::Parentheses>(
+ "(" >> expr / !","_tok / recovery(")"_tok, SkipPast<')'>{}))),
construct<Expr>(indirect(functionReference) / !"("_tok / !"%"_tok),
construct<Expr>(designator / !"("_tok / !"%"_tok),
construct<Expr>(indirect(Parser<SubstringInquiry>{})), // %LEN or %KIND
diff --git a/flang/lib/Parser/program-parsers.cpp b/flang/lib/Parser/program-parsers.cpp
index 6f25ba4827220..b51b60157f39c 100644
--- a/flang/lib/Parser/program-parsers.cpp
+++ b/flang/lib/Parser/program-parsers.cpp
@@ -86,10 +86,15 @@ TYPE_CONTEXT_PARSER("specification part"_en_US,
// are in contexts that impose constraints on the kinds of statements that
// are allowed, and so we have a variant production for declaration-construct
// that implements those constraints.
-constexpr auto execPartLookAhead{first(actionStmt >> ok, openaccConstruct >> ok,
- openmpConstruct >> ok, "ASSOCIATE ("_tok, "BLOCK"_tok, "SELECT"_tok,
- "CHANGE TEAM"_sptok, "CRITICAL"_tok, "DO"_tok, "IF ("_tok, "WHERE ("_tok,
- "FORALL ("_tok, "!$CUF"_tok)};
+constexpr auto actionStmtLookAhead{first(actionStmt >> ok,
+ // Also accept apparent action statements with errors if they might be
+ // first in the execution part
+ "ALLOCATE ("_tok, "CALL" >> name >> "("_tok, "GO TO"_tok, "OPEN ("_tok,
+ "PRINT"_tok / space / !"("_tok, "READ ("_tok, "WRITE ("_tok)};
+constexpr auto execPartLookAhead{first(actionStmtLookAhead >> ok,
+ openaccConstruct >> ok, openmpConstruct >> ok, "ASSOCIATE ("_tok,
+ "BLOCK"_tok, "SELECT"_tok, "CHANGE TEAM"_sptok, "CRITICAL"_tok, "DO"_tok,
+ "IF ("_tok, "WHERE ("_tok, "FORALL ("_tok, "!$CUF"_tok)};
constexpr auto declErrorRecovery{
stmtErrorRecoveryStart >> !execPartLookAhead >> skipStmtErrorRecovery};
constexpr auto misplacedSpecificationStmt{Parser<UseStmt>{} >>
@@ -446,10 +451,13 @@ TYPE_PARSER(extension<LanguageFeature::CUDA>(
"<<<" >> construct<CallStmt::Chevrons>(scalarExpr, "," >> scalarExpr,
maybe("," >> scalarIntExpr), maybe("," >> scalarIntExpr)) /
">>>"))
-TYPE_PARSER(construct<CallStmt>(
- sourced(construct<CallStmt>("CALL" >> Parser<ProcedureDesignator>{},
- maybe(Parser<CallStmt::Chevrons>{}),
- defaulted(parenthesized(optionalList(actualArgSpec)))))))
+constexpr auto actualArgSpecList{optionalList(actualArgSpec)};
+TYPE_CONTEXT_PARSER("CALL statement"_en_US,
+ construct<CallStmt>(
+ sourced(construct<CallStmt>("CALL" >> Parser<ProcedureDesignator>{},
+ maybe(Parser<CallStmt::Chevrons>{}) / space,
+ "(" >> actualArgSpecList / ")" ||
+ lookAhead(endOfStmt) >> defaulted(actualArgSpecList)))))
// R1522 procedure-designator ->
// procedure-name | proc-component-ref | data-ref % binding-name
diff --git a/flang/lib/Parser/token-parsers.h b/flang/lib/Parser/token-parsers.h
index 2495017d19649..4127a973f22e5 100644
--- a/flang/lib/Parser/token-parsers.h
+++ b/flang/lib/Parser/token-parsers.h
@@ -560,6 +560,8 @@ template <char goal> struct SkipPast {
while (std::optional<const char *> p{state.GetNextChar()}) {
if (**p == goal) {
return {Success{}};
+ } else if (**p == '\n') {
+ break;
}
}
return std::nullopt;
@@ -574,8 +576,11 @@ template <char goal> struct SkipTo {
while (std::optional<const char *> p{state.PeekAtNextChar()}) {
if (**p == goal) {
return {Success{}};
+ } else if (**p == '\n') {
+ break;
+ } else {
+ state.UncheckedAdvance();
}
- state.UncheckedAdvance();
}
return std::nullopt;
}
diff --git a/flang/test/Parser/recovery01.f90 b/flang/test/Parser/recovery01.f90
new file mode 100644
index 0000000000000..674abaccc7c7d
--- /dev/null
+++ b/flang/test/Parser/recovery01.f90
@@ -0,0 +1,10 @@
+! RUN: not %flang_fc1 -fsyntax-only %s 2>&1 | FileCheck %s
+program main
+ call foo(i, &
+ j, &
+ k, &
+ 1$)
+end
+
+!CHECK: error: expected ')'
+!CHECK: in the context: CALL statement
diff --git a/flang/test/Parser/recovery02.f90 b/flang/test/Parser/recovery02.f90
new file mode 100644
index 0000000000000..0d0d15545cf39
--- /dev/null
+++ b/flang/test/Parser/recovery02.f90
@@ -0,0 +1,8 @@
+! RUN: not %flang_fc1 -fsyntax-only %s 2>&1 | FileCheck %s
+continue ! force executable part
+CALL ADD_HASH_BLOCK(d_c,f_c,dimc, &
+ (h2b-1+noab*(h1b-1+noab*(p4b-noab-1+nvab*(p3b-noab-1$)))))
+end
+
+!CHECK: error: expected ')'
+!CHECK: in the context: CALL statement
More information about the flang-commits
mailing list