[flang-commits] [flang] [flang][parser] Better error recovery for misplaced declaration (PR #100482)

[via flang-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-flang-parser

Author: Peter Klausler (klausler)

<details>
<summary>Changes</summary>

When a declaration construct appears in the execution part of a block or subprogram body, report it as such rather than as a misleading syntax error on the executable statement that it somehow matched the most.

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


3 Files Affected:

- (modified) flang/lib/Parser/executable-parsers.cpp (+12-13) 
- (modified) flang/lib/Parser/program-parsers.cpp (+23-22) 
- (added) flang/test/Parser/recovery03.f90 (+9) 


``````````diff
diff --git a/flang/lib/Parser/executable-parsers.cpp b/flang/lib/Parser/executable-parsers.cpp
index f703e09612d54..5057e89164c9f 100644
--- a/flang/lib/Parser/executable-parsers.cpp
+++ b/flang/lib/Parser/executable-parsers.cpp
@@ -67,20 +67,19 @@ constexpr auto obsoleteExecutionPartConstruct{recovery(ignoredStatementPrefix >>
                 parenthesized(nonemptyList(Parser<AllocateShapeSpec>{}))))))};
 
 TYPE_PARSER(recovery(
-    withMessage("expected execution part construct"_err_en_US,
-        CONTEXT_PARSER("execution part construct"_en_US,
-            first(construct<ExecutionPartConstruct>(executableConstruct),
+    CONTEXT_PARSER("execution part construct"_en_US,
+        first(construct<ExecutionPartConstruct>(executableConstruct),
+            construct<ExecutionPartConstruct>(statement(indirect(formatStmt))),
+            construct<ExecutionPartConstruct>(statement(indirect(entryStmt))),
+            construct<ExecutionPartConstruct>(statement(indirect(dataStmt))),
+            extension<LanguageFeature::ExecutionPartNamelist>(
+                "nonstandard usage: NAMELIST in execution part"_port_en_US,
                 construct<ExecutionPartConstruct>(
-                    statement(indirect(formatStmt))),
-                construct<ExecutionPartConstruct>(
-                    statement(indirect(entryStmt))),
-                construct<ExecutionPartConstruct>(
-                    statement(indirect(dataStmt))),
-                extension<LanguageFeature::ExecutionPartNamelist>(
-                    "nonstandard usage: NAMELIST in execution part"_port_en_US,
-                    construct<ExecutionPartConstruct>(
-                        statement(indirect(Parser<NamelistStmt>{})))),
-                obsoleteExecutionPartConstruct))),
+                    statement(indirect(Parser<NamelistStmt>{})))),
+            obsoleteExecutionPartConstruct,
+            lookAhead(declarationConstruct) >> SkipTo<'\n'>{} >>
+                fail<ExecutionPartConstruct>(
+                    "misplaced declaration in the execution part"_err_en_US))),
     construct<ExecutionPartConstruct>(executionPartErrorRecovery)))
 
 // R509 execution-part -> executable-construct [execution-part-construct]...
diff --git a/flang/lib/Parser/program-parsers.cpp b/flang/lib/Parser/program-parsers.cpp
index b51b60157f39c..5b8fe8c42b179 100644
--- a/flang/lib/Parser/program-parsers.cpp
+++ b/flang/lib/Parser/program-parsers.cpp
@@ -66,16 +66,6 @@ TYPE_PARSER(
             normalProgramUnit) /
             skipStuffBeforeStatement))
 
-// R504 specification-part ->
-//         [use-stmt]... [import-stmt]... [implicit-part]
-//         [declaration-construct]...
-TYPE_CONTEXT_PARSER("specification part"_en_US,
-    construct<SpecificationPart>(many(openaccDeclarativeConstruct),
-        many(openmpDeclarativeConstruct), many(indirect(compilerDirective)),
-        many(statement(indirect(Parser<UseStmt>{}))),
-        many(unambiguousStatement(indirect(Parser<ImportStmt>{}))),
-        implicitPart, many(declarationConstruct)))
-
 // R507 declaration-construct ->
 //        specification-construct | data-stmt | format-stmt |
 //        entry-stmt | stmt-function-stmt
@@ -106,18 +96,29 @@ constexpr auto misplacedSpecificationStmt{Parser<UseStmt>{} >>
         fail<DeclarationConstruct>(
             "IMPLICIT statements must follow USE and IMPORT and precede all other declarations"_err_en_US)};
 
-TYPE_PARSER(recovery(
-    withMessage("expected declaration construct"_err_en_US,
-        CONTEXT_PARSER("declaration construct"_en_US,
-            first(construct<DeclarationConstruct>(specificationConstruct),
-                construct<DeclarationConstruct>(statement(indirect(dataStmt))),
-                construct<DeclarationConstruct>(
-                    statement(indirect(formatStmt))),
-                construct<DeclarationConstruct>(statement(indirect(entryStmt))),
-                construct<DeclarationConstruct>(
-                    statement(indirect(Parser<StmtFunctionStmt>{}))),
-                misplacedSpecificationStmt))),
-    construct<DeclarationConstruct>(declErrorRecovery)))
+TYPE_CONTEXT_PARSER("declaration construct"_en_US,
+    first(construct<DeclarationConstruct>(specificationConstruct),
+        construct<DeclarationConstruct>(statement(indirect(dataStmt))),
+        construct<DeclarationConstruct>(statement(indirect(formatStmt))),
+        construct<DeclarationConstruct>(statement(indirect(entryStmt))),
+        construct<DeclarationConstruct>(
+            statement(indirect(Parser<StmtFunctionStmt>{}))),
+        misplacedSpecificationStmt))
+
+constexpr auto recoveredDeclarationConstruct{
+    recovery(withMessage("expected declaration construct"_err_en_US,
+                 declarationConstruct),
+        construct<DeclarationConstruct>(declErrorRecovery))};
+
+// R504 specification-part ->
+//         [use-stmt]... [import-stmt]... [implicit-part]
+//         [declaration-construct]...
+TYPE_CONTEXT_PARSER("specification part"_en_US,
+    construct<SpecificationPart>(many(openaccDeclarativeConstruct),
+        many(openmpDeclarativeConstruct), many(indirect(compilerDirective)),
+        many(statement(indirect(Parser<UseStmt>{}))),
+        many(unambiguousStatement(indirect(Parser<ImportStmt>{}))),
+        implicitPart, many(recoveredDeclarationConstruct)))
 
 // R507 variant of declaration-construct for use in limitedSpecificationPart.
 constexpr auto invalidDeclarationStmt{formatStmt >>
diff --git a/flang/test/Parser/recovery03.f90 b/flang/test/Parser/recovery03.f90
new file mode 100644
index 0000000000000..f3340f0d99e0e
--- /dev/null
+++ b/flang/test/Parser/recovery03.f90
@@ -0,0 +1,9 @@
+! RUN: not %flang_fc1 -fsyntax-only %s 2>&1 | FileCheck %s
+! CHECK: error: misplaced declaration in the execution part
+! CHECK:  real, pointer :: p2(:,:)
+! CHECK: in the context: execution part construct
+real, allocatable, target :: a2(:,:)
+allocate(a2(2:11,0:9))
+real, pointer :: p2(:,:)
+p2 => a2(2:3,1:2)
+end

``````````

</details>


https://github.com/llvm/llvm-project/pull/100482