[flang-commits] [flang] 5260132 - [flang] Improve syntax error messages by fixing withMessage() parser combinator
Peter Klausler via flang-commits
flang-commits at lists.llvm.org
Thu Oct 6 14:00:22 PDT 2022
Author: Peter Klausler
Date: 2022-10-06T14:00:06-07:00
New Revision: 52601325f1a4db06510dbe12562240a018a254bd
URL: https://github.com/llvm/llvm-project/commit/52601325f1a4db06510dbe12562240a018a254bd
DIFF: https://github.com/llvm/llvm-project/commit/52601325f1a4db06510dbe12562240a018a254bd.diff
LOG: [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
Added:
flang/test/Parser/doubled-comma.f90
Modified:
flang/lib/Parser/basic-parsers.h
flang/test/Driver/color-diagnostics-parse.f90
flang/test/Semantics/error_stop1a.f90
flang/test/Semantics/synchronization01a.f90
flang/test/Semantics/synchronization03a.f90
Removed:
################################################################################
diff --git a/flang/lib/Parser/basic-parsers.h b/flang/lib/Parser/basic-parsers.h
index e2355a7526ae8..784bd770fa64f 100644
--- a/flang/lib/Parser/basic-parsers.h
+++ b/flang/lib/Parser/basic-parsers.h
@@ -208,26 +208,31 @@ template <typename PA> class WithMessageParser {
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 @@ template <typename PA, typename PB> class RecoveryParser {
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 @@ template <typename PA, typename PB> class RecoveryParser {
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 @@ template <typename PA, typename PB> class RecoveryParser {
private:
const PA pa_;
- const PB pb3_;
+ const PB pb_;
};
template <typename PA, typename PB>
diff --git a/flang/test/Driver/color-diagnostics-parse.f90 b/flang/test/Driver/color-diagnostics-parse.f90
index 3d8d3a3c0c602..11a1c7b57c9e2 100644
--- a/flang/test/Driver/color-diagnostics-parse.f90
+++ b/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 =
diff --git a/flang/test/Parser/doubled-comma.f90 b/flang/test/Parser/doubled-comma.f90
new file mode 100644
index 0000000000000..f52feb068ca38
--- /dev/null
+++ b/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
diff --git a/flang/test/Semantics/error_stop1a.f90 b/flang/test/Semantics/error_stop1a.f90
index cb90d9644c93c..f145ccbbe2e7a 100644
--- a/flang/test/Semantics/error_stop1a.f90
+++ b/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
diff --git a/flang/test/Semantics/synchronization01a.f90 b/flang/test/Semantics/synchronization01a.f90
index 4d4b68e26460e..ce19cb9e60fc7 100644
--- a/flang/test/Semantics/synchronization01a.f90
+++ b/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
diff --git a/flang/test/Semantics/synchronization03a.f90 b/flang/test/Semantics/synchronization03a.f90
index 11171e945a9f2..db885a84dbfb7 100644
--- a/flang/test/Semantics/synchronization03a.f90
+++ b/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
More information about the flang-commits
mailing list