[flang-commits] [flang] 16ecf40 - [flang] Improve parser error recovery (#173803)
via flang-commits
flang-commits at lists.llvm.org
Wed Dec 31 12:43:32 PST 2025
Author: Peter Klausler
Date: 2025-12-31T12:43:28-08:00
New Revision: 16ecf40aea105c3cf908b9653d6556e31bbc480a
URL: https://github.com/llvm/llvm-project/commit/16ecf40aea105c3cf908b9653d6556e31bbc480a
DIFF: https://github.com/llvm/llvm-project/commit/16ecf40aea105c3cf908b9653d6556e31bbc480a.diff
LOG: [flang] Improve parser error recovery (#173803)
A bad I/O control specification ("NML=123" in this case) triggers an
assertion when parsing fails but emits no error message. The message was
lost when combining failed alternative parse states. Make
CombineFailedParses() take more care to not discard messages; then also
strengthen I/O control specification error recovery so that the original
test case doesn't just elicit a confusing complaint about a missing
expected "*" at the site of the "NML" keyword.
Fixes https://github.com/llvm/llvm-project/issues/173522.
Added:
flang/test/Parser/bug173522.f90
Modified:
flang/include/flang/Parser/parse-state.h
flang/include/flang/Parser/parse-tree.h
flang/lib/Lower/IO.cpp
flang/lib/Parser/basic-parsers.h
flang/lib/Parser/io-parsers.cpp
flang/lib/Parser/unparse.cpp
Removed:
################################################################################
diff --git a/flang/include/flang/Parser/parse-state.h b/flang/include/flang/Parser/parse-state.h
index 36d70b81b7923..cc787e4105b55 100644
--- a/flang/include/flang/Parser/parse-state.h
+++ b/flang/include/flang/Parser/parse-state.h
@@ -192,19 +192,29 @@ class ParseState {
return remain;
}
- void CombineFailedParses(ParseState &&prev) {
- if (prev.anyTokenMatched_) {
- if (!anyTokenMatched_ || prev.p_ > p_) {
- anyTokenMatched_ = true;
- p_ = prev.p_;
- messages_ = std::move(prev.messages_);
- } else if (prev.p_ == p_) {
- messages_.Merge(std::move(prev.messages_));
- }
+ void CombineFailedParses(ParseState &&that) {
+ bool haveMessages{anyDeferredMessages_ || !messages_.empty()};
+ bool thatHasMessages{that.anyDeferredMessages_ || !that.messages_.empty()};
+ if (haveMessages && !thatHasMessages) {
+ // never discard messages
+ } else if (!that.anyTokenMatched_ && (haveMessages || !thatHasMessages)) {
+ // token matching is preferred
+ } else if ((!anyTokenMatched_ && that.anyTokenMatched_) ||
+ (!haveMessages && thatHasMessages) ||
+ that.p_ > p_) { // 'that' is better, or no worse and got further
+ anyTokenMatched_ = that.anyTokenMatched_;
+ p_ = that.p_;
+ messages_ = std::move(that.messages_);
+ anyDeferredMessages_ = that.anyDeferredMessages_;
+ anyConformanceViolation_ = that.anyConformanceViolation_;
+ anyErrorRecovery_ = that.anyErrorRecovery_;
+ } else if (that.p_ == p_) { // merge states
+ anyTokenMatched_ |= that.anyTokenMatched_;
+ messages_.Merge(std::move(that.messages_));
+ anyDeferredMessages_ |= that.anyDeferredMessages_;
+ anyConformanceViolation_ |= that.anyConformanceViolation_;
+ anyErrorRecovery_ |= that.anyErrorRecovery_;
}
- anyDeferredMessages_ |= prev.anyDeferredMessages_;
- anyConformanceViolation_ |= prev.anyConformanceViolation_;
- anyErrorRecovery_ |= prev.anyErrorRecovery_;
}
private:
diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index 8712f861b82d8..2ab640ed351d8 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -2749,7 +2749,8 @@ struct IoControlSpec {
WRAPPER_CLASS(Rec, ScalarIntExpr);
WRAPPER_CLASS(Size, ScalarIntVariable);
std::variant<IoUnit, Format, Name, CharExpr, Asynchronous, EndLabel, EorLabel,
- ErrLabel, IdVariable, MsgVariable, StatVariable, Pos, Rec, Size>
+ ErrLabel, IdVariable, MsgVariable, StatVariable, Pos, Rec, Size,
+ ErrorRecovery>
u;
};
diff --git a/flang/lib/Lower/IO.cpp b/flang/lib/Lower/IO.cpp
index cd53dc9482283..c3f9c1a882423 100644
--- a/flang/lib/Lower/IO.cpp
+++ b/flang/lib/Lower/IO.cpp
@@ -1427,6 +1427,9 @@ static void threadSpecs(Fortran::lower::AbstractConverter &converter,
// already finished.
return ok;
},
+ [](const Fortran::parser::ErrorRecovery &) -> mlir::Value {
+ llvm::report_fatal_error("ErrorRecovery in parse tree");
+ },
[&](const auto &x) {
return genIOOption(converter, loc, cookie, x);
}},
diff --git a/flang/lib/Parser/basic-parsers.h b/flang/lib/Parser/basic-parsers.h
index 46d5168c80fe7..eeb59a830fc0c 100644
--- a/flang/lib/Parser/basic-parsers.h
+++ b/flang/lib/Parser/basic-parsers.h
@@ -395,7 +395,7 @@ template <typename PA, typename PB> class RecoveryParser {
}
if (bx) {
// Error recovery situations must also produce messages.
- CHECK(state.anyDeferredMessages() || state.messages().AnyFatalError());
+ CHECK(hadDeferredMessages || state.messages().AnyFatalError());
state.set_anyErrorRecovery();
}
return bx;
diff --git a/flang/lib/Parser/io-parsers.cpp b/flang/lib/Parser/io-parsers.cpp
index c69ea58738b90..cb3e68a05c94d 100644
--- a/flang/lib/Parser/io-parsers.cpp
+++ b/flang/lib/Parser/io-parsers.cpp
@@ -231,7 +231,12 @@ TYPE_PARSER(first(construct<IoControlSpec>("UNIT =" >> ioUnit),
construct<IoControlSpec::CharExpr>(
pure(IoControlSpec::CharExpr::Kind::Sign), scalarDefaultCharExpr)),
construct<IoControlSpec>(
- "SIZE =" >> construct<IoControlSpec::Size>(scalarIntVariable))))
+ "SIZE =" >> construct<IoControlSpec::Size>(scalarIntVariable)),
+ lookAhead(keyword) >>
+ construct<IoControlSpec>(recovery(
+ fail<ErrorRecovery>(
+ "invalid or unknown I/O control specification"_err_en_US),
+ keyword >> "="_tok >> expr >> construct<ErrorRecovery>()))))
// R1211 write-stmt -> WRITE ( io-control-spec-list ) [output-item-list]
constexpr auto outputItemList{
diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp
index 1164dfd0bdf50..0a6b0cdf88efa 100644
--- a/flang/lib/Parser/unparse.cpp
+++ b/flang/lib/Parser/unparse.cpp
@@ -1326,65 +1326,26 @@ class UnparseVisitor {
Walk(", ", std::get<std::list<OutputItem>>(x.t), ", ");
}
bool Pre(const IoControlSpec &x) { // R1213
- return common::visit(common::visitors{
- [&](const IoUnit &) {
- Word("UNIT=");
- return true;
- },
- [&](const Format &) {
- Word("FMT=");
- return true;
- },
- [&](const Name &) {
- Word("NML=");
- return true;
- },
- [&](const IoControlSpec::CharExpr &y) {
- Walk(y.t, "=");
- return false;
- },
- [&](const IoControlSpec::Asynchronous &) {
- Word("ASYNCHRONOUS=");
- return true;
- },
- [&](const EndLabel &) {
- Word("END=");
- return true;
- },
- [&](const EorLabel &) {
- Word("EOR=");
- return true;
- },
- [&](const ErrLabel &) {
- Word("ERR=");
- return true;
- },
- [&](const IdVariable &) {
- Word("ID=");
- return true;
- },
- [&](const MsgVariable &) {
- Word("IOMSG=");
- return true;
- },
- [&](const StatVariable &) {
- Word("IOSTAT=");
- return true;
- },
- [&](const IoControlSpec::Pos &) {
- Word("POS=");
- return true;
- },
- [&](const IoControlSpec::Rec &) {
- Word("REC=");
- return true;
- },
- [&](const IoControlSpec::Size &) {
- Word("SIZE=");
- return true;
- },
- },
+ common::visit(
+ common::visitors{
+ [&](const IoUnit &) { Word("UNIT="); },
+ [&](const Format &) { Word("FMT="); },
+ [&](const Name &) { Word("NML="); },
+ [&](const IoControlSpec::CharExpr &y) { Walk(y.t, "="); },
+ [&](const IoControlSpec::Asynchronous &) { Word("ASYNCHRONOUS="); },
+ [&](const EndLabel &) { Word("END="); },
+ [&](const EorLabel &) { Word("EOR="); },
+ [&](const ErrLabel &) { Word("ERR="); },
+ [&](const IdVariable &) { Word("ID="); },
+ [&](const MsgVariable &) { Word("IOMSG="); },
+ [&](const StatVariable &) { Word("IOSTAT="); },
+ [&](const IoControlSpec::Pos &) { Word("POS="); },
+ [&](const IoControlSpec::Rec &) { Word("REC="); },
+ [&](const IoControlSpec::Size &) { Word("SIZE="); },
+ [&](const ErrorRecovery &) {},
+ },
x.u);
+ return true;
}
void Unparse(const InputImpliedDo &x) { // R1218
Put('('), Walk(std::get<std::list<InputItem>>(x.t), ", "), Put(", ");
diff --git a/flang/test/Parser/bug173522.f90 b/flang/test/Parser/bug173522.f90
new file mode 100644
index 0000000000000..bb8321be34f36
--- /dev/null
+++ b/flang/test/Parser/bug173522.f90
@@ -0,0 +1,4 @@
+!RUN: not %flang_fc1 -fsyntax-only %s 2>&1 | FileCheck %s
+!CHECK: 3:10: error: invalid or unknown I/O control specification
+write (*,nml=123)
+end
More information about the flang-commits
mailing list