[flang][parser] Better error recovery for SUBROUTINE/FUNCTION statements

[klausler]

When there's an error in a SUBROUTINE or FUNCTION statement, errors cascade quickly because the body of the subprogram or interface isn't in the right context. So, if a SUBROUTINE or FUNCTION statement is expected, and contains a SUBROUTINE or FUNCTION keyword, it counts as one -- retain and emit any errors pertaining to the arguments or suffix, recover to the end of the line if needed, and proceed.

[llvmbot]

@llvm/pr-subscribers-flang-parser

Author: Peter Klausler (klausler)

Changes

When there's an error in a SUBROUTINE or FUNCTION statement, errors cascade quickly because the body of the subprogram or interface isn't in the right context. So, if a SUBROUTINE or FUNCTION statement is expected, and contains a SUBROUTINE or FUNCTION keyword, it counts as one -- retain and emit any errors pertaining to the arguments or suffix, recover to the end of the line if needed, and proceed.

---

Full diff: https://github.com/llvm/llvm-project/pull/100664.diff

3 Files Affected:

• (modified) flang/lib/Parser/program-parsers.cpp (+14-14)
• (modified) flang/lib/Parser/stmt-parser.h (+5)
• (added) flang/test/Parser/recovery04.f90 (+24)

diff --git a/flang/lib/Parser/program-parsers.cpp b/flang/lib/Parser/program-parsers.cpp
index b51b60157f39c..13770c0b16489 100644
--- a/flang/lib/Parser/program-parsers.cpp
+++ b/flang/lib/Parser/program-parsers.cpp
@@ -532,15 +532,16 @@ TYPE_CONTEXT_PARSER("FUNCTION subprogram"_en_US,
 //         [prefix] FUNCTION function-name ( [dummy-arg-name-list] ) [suffix]
 // R1526 prefix -> prefix-spec [prefix-spec]...
 // R1531 dummy-arg-name -> name
-TYPE_CONTEXT_PARSER("FUNCTION statement"_en_US,
+TYPE_PARSER(
     construct<FunctionStmt>(many(prefixSpec), "FUNCTION" >> name,
-        parenthesized(optionalList(name)), maybe(suffix)) ||
-        extension<LanguageFeature::OmitFunctionDummies>(
-            "nonstandard usage: FUNCTION statement without dummy argument list"_port_en_US,
-            construct<FunctionStmt>( // PGI & Intel accept "FUNCTION F"
-                many(prefixSpec), "FUNCTION" >> name,
-                construct<std::list<Name>>(),
-                construct<std::optional<Suffix>>())))
+        // PGI & Intel accept "FUNCTION F"
+        !"("_tok >>
+                extension<LanguageFeature::OmitFunctionDummies>(
+                    "nonstandard usage: FUNCTION statement without dummy argument list"_port_en_US,
+                    pure<std::list<Name>>()) ||
+            defaulted(parenthesized(optionalList(name))),
+        maybe(suffix)) /
+    checkEndOfKnownStmt)
 
 // R1532 suffix ->
 //         proc-language-binding-spec [RESULT ( result-name )] |
@@ -565,12 +566,11 @@ TYPE_CONTEXT_PARSER("SUBROUTINE subprogram"_en_US,
 // R1535 subroutine-stmt ->
 //         [prefix] SUBROUTINE subroutine-name [( [dummy-arg-list] )
 //         [proc-language-binding-spec]]
-TYPE_PARSER(
-    construct<SubroutineStmt>(many(prefixSpec), "SUBROUTINE" >> name,
-        parenthesized(optionalList(dummyArg)), maybe(languageBindingSpec)) ||
-    construct<SubroutineStmt>(many(prefixSpec), "SUBROUTINE" >> name,
-        pure<std::list<DummyArg>>(),
-        pure<std::optional<LanguageBindingSpec>>()))
+TYPE_PARSER(construct<SubroutineStmt>(many(prefixSpec), "SUBROUTINE" >> name,
+                !"("_tok >> pure<std::list<DummyArg>>() ||
+                    defaulted(parenthesized(optionalList(dummyArg))),
+                maybe(languageBindingSpec)) /
+    checkEndOfKnownStmt)
 
 // R1536 dummy-arg -> dummy-arg-name | *
 TYPE_PARSER(construct<DummyArg>(name) || construct<DummyArg>(star))
diff --git a/flang/lib/Parser/stmt-parser.h b/flang/lib/Parser/stmt-parser.h
index ba647fd60d4ae..dc5769f32219e 100644
--- a/flang/lib/Parser/stmt-parser.h
+++ b/flang/lib/Parser/stmt-parser.h
@@ -30,6 +30,11 @@ inline constexpr auto unterminatedStatement(const PA &p) {
           maybe(label), space >> p));
 }
 
+constexpr auto checkEndOfKnownStmt{
+    recovery(withMessage("expected end of statement"_err_en_US,
+                 space >> lookAhead(";\n"_ch)),
+        SkipTo<'\n'>{})};
+
 constexpr auto endOfLine{
     "\n"_ch >> ok || fail("expected end of line"_err_en_US)};
 
diff --git a/flang/test/Parser/recovery04.f90 b/flang/test/Parser/recovery04.f90
new file mode 100644
index 0000000000000..144ebd24f71b5
--- /dev/null
+++ b/flang/test/Parser/recovery04.f90
@@ -0,0 +1,24 @@
+! RUN: not %flang_fc1 -fsyntax-only %s 2>&1 | FileCheck %s
+module m
+ contains
+  !CHECK: expected end of statement
+  !CHECK: subroutine s1(var i, j)
+  subroutine s1(var i, j)
+  end subroutine
+  !CHECK: expected end of statement
+  !CHECK: subroutine s2[b]
+  subroutine s2[b]
+  end subroutine
+  !CHECK: expected end of statement
+  !CHECK: function f1(var i, j)
+  function f1(var i, j)
+  end function
+  !CHECK: expected end of statement
+  !CHECK: function f2[b]
+  function f2[b]
+  end function
+  !CHECK: expected end of statement
+  !CHECK: function f3(a,*)
+  function f3(a,*)
+  end function
+end

[psteinfeld]

All builds and tests correctly and looks good.