[flang] Intermix messages from parser and semantic analysis (#90654)

When there are one or more fatal error messages produced by the parser,
semantic analysis is not performed. But when there are messages produced
by the parser and none of them are fatal, those messages are emitted to
the user before compilation continues with semantic analysis, and any
messages produced by semantics are emitted after the messages from
parsing.

This can be confusing for the user, as the messages may no longer all be
in source file location order. It also makes it difficult to write tests
that check for both non-fatal messages from parsing as well as messages
from semantics using inline CHECK: or other expected messages in test
source code.

This patch ensures that if semantic analysis is performed, and non-fatal
messages were produced by the parser, that all the messages will be
combined and emitted in source file order.

Author: klausler

flang/examples/FeatureList/FeatureList.cpp

@@ -775,7 +775,9 @@ class FeatureListAction : public PluginParseTreeAction {
     }
   }
 
-  bool beginSourceFileAction() override { return runPrescan() && runParse(); }
+  bool beginSourceFileAction() override {
+    return runPrescan() && runParse(/*emitMessages=*/true);
+  }
 };
 
 static FrontendPluginRegistry::Add<FeatureListAction> X(

flang/include/flang/Frontend/FrontendAction.h

@@ -112,7 +112,7 @@ class FrontendAction {
   bool runPrescan();
   // Parse the current input file. Return False if fatal errors are reported,
   // True otherwise.
-  bool runParse();
+  bool runParse(bool emitMessages);
   // Run semantic checks for the current input file. Return False if fatal
   // errors are reported, True otherwise.
   bool runSemanticChecks();

flang/include/flang/Semantics/semantics.h

@@ -312,7 +312,7 @@ class Semantics {
     return context_.FindScope(where);
   }
   bool AnyFatalError() const { return context_.AnyFatalError(); }
-  void EmitMessages(llvm::raw_ostream &) const;
+  void EmitMessages(llvm::raw_ostream &);
   void DumpSymbols(llvm::raw_ostream &);
   void DumpSymbolsSources(llvm::raw_ostream &) const;
 

flang/lib/Frontend/FrontendAction.cpp

@@ -153,7 +153,7 @@ bool FrontendAction::runPrescan() {
   return !reportFatalScanningErrors();
 }
 
-bool FrontendAction::runParse() {
+bool FrontendAction::runParse(bool emitMessages) {
   CompilerInstance &ci = this->getInstance();
 
   // Parse. In case of failure, report and return.
@@ -163,9 +163,11 @@ bool FrontendAction::runParse() {
     return false;
   }
 
-  // Report the diagnostics from getParsing
-  ci.getParsing().messages().Emit(llvm::errs(), ci.getAllCookedSources());
-
+  if (emitMessages) {
+    // Report any non-fatal diagnostics from getParsing now rather than
+    // combining them with messages from semantics.
+    ci.getParsing().messages().Emit(llvm::errs(), ci.getAllCookedSources());
+  }
   return true;
 }
 
@@ -174,10 +176,14 @@ bool FrontendAction::runSemanticChecks() {
   std::optional<parser::Program> &parseTree{ci.getParsing().parseTree()};
   assert(parseTree && "Cannot run semantic checks without a parse tree!");
 
+  // Transfer any pending non-fatal messages from parsing to semantics
+  // so that they are merged and all printed in order.
+  auto &semanticsCtx{ci.getSemanticsContext()};
+  semanticsCtx.messages().Annex(std::move(ci.getParsing().messages()));
+
   // Prepare semantics
   ci.setSemantics(std::make_unique<Fortran::semantics::Semantics>(
-      ci.getSemanticsContext(), *parseTree,
-      ci.getInvocation().getDebugModuleDir()));
+      semanticsCtx, *parseTree, ci.getInvocation().getDebugModuleDir()));
   auto &semantics = ci.getSemantics();
 
   // Run semantic checks
@@ -187,7 +193,7 @@ bool FrontendAction::runSemanticChecks() {
     return false;
   }
 
-  // Report the diagnostics from the semantic checks
+  // Report the diagnostics from parsing and the semantic checks
   semantics.EmitMessages(ci.getSemaOutputStream());
 
   return true;

flang/lib/Frontend/FrontendActions.cpp

@@ -123,12 +123,12 @@ static bool saveMLIRTempFile(const CompilerInvocation &ci,
 bool PrescanAction::beginSourceFileAction() { return runPrescan(); }
 
 bool PrescanAndParseAction::beginSourceFileAction() {
-  return runPrescan() && runParse();
+  return runPrescan() && runParse(/*emitMessages=*/true);
 }
 
 bool PrescanAndSemaAction::beginSourceFileAction() {
-  return runPrescan() && runParse() && runSemanticChecks() &&
-         generateRtTypeTables();
+  return runPrescan() && runParse(/*emitMessages=*/false) &&
+         runSemanticChecks() && generateRtTypeTables();
 }
 
 bool PrescanAndSemaDebugAction::beginSourceFileAction() {
@@ -137,8 +137,8 @@ bool PrescanAndSemaDebugAction::beginSourceFileAction() {
   // from exiting early (i.e. in the presence of semantic errors). We should
   // never do this in actions intended for end-users or otherwise regular
   // compiler workflows!
-  return runPrescan() && runParse() && (runSemanticChecks() || true) &&
-         (generateRtTypeTables() || true);
+  return runPrescan() && runParse(/*emitMessages=*/false) &&
+         (runSemanticChecks() || true) && (generateRtTypeTables() || true);
 }
 
 static void addDependentLibs(mlir::ModuleOp &mlirModule, CompilerInstance &ci) {
@@ -275,8 +275,8 @@ bool CodeGenAction::beginSourceFileAction() {
     ci.getDiagnostics().Report(diagID);
     return false;
   }
-  bool res = runPrescan() && runParse() && runSemanticChecks() &&
-             generateRtTypeTables();
+  bool res = runPrescan() && runParse(/*emitMessages=*/false) &&
+             runSemanticChecks() && generateRtTypeTables();
   if (!res)
     return res;
 

flang/lib/Semantics/semantics.cpp

@@ -593,7 +593,10 @@ bool Semantics::Perform() {
       ModFileWriter{context_}.WriteAll();
 }
 
-void Semantics::EmitMessages(llvm::raw_ostream &os) const {
+void Semantics::EmitMessages(llvm::raw_ostream &os) {
+  // Resolve the CharBlock locations of the Messages to ProvenanceRanges
+  // so messages from parsing and semantics are intermixed in source order.
+  context_.messages().ResolveProvenances(context_.allCookedSources());
   context_.messages().Emit(os, context_.allCookedSources());
 }
 

flang/test/Driver/message-merging.f90

@@ -0,0 +1,7 @@
+!RUN: %flang -fsyntax-only -pedantic -I %S/Inputs/ %s 2>&1 | FileCheck %s
+!CHECK: warning: SAVE attribute was already specified on 'x'
+!CHECK: portability: #include: extra stuff ignored after file name
+save x
+save x
+#include <empty.h>    crud after header name
+end

flang/tools/bbc/bbc.cpp

@@ -289,17 +289,20 @@ static mlir::LogicalResult convertFortranSourceToMLIR(
 
   // parse the input Fortran
   parsing.Parse(llvm::outs());
-  parsing.messages().Emit(llvm::errs(), parsing.allCooked());
   if (!parsing.consumedWholeFile()) {
+    parsing.messages().Emit(llvm::errs(), parsing.allCooked());
     parsing.EmitMessage(llvm::errs(), parsing.finalRestingPlace(),
                         "parser FAIL (final position)",
                         "error: ", llvm::raw_ostream::RED);
     return mlir::failure();
-  }
-  if ((!parsing.messages().empty() && (parsing.messages().AnyFatalError())) ||
-      !parsing.parseTree().has_value()) {
+  } else if ((!parsing.messages().empty() &&
+              (parsing.messages().AnyFatalError())) ||
+             !parsing.parseTree().has_value()) {
+    parsing.messages().Emit(llvm::errs(), parsing.allCooked());
     llvm::errs() << programPrefix << "could not parse " << path << '\n';
     return mlir::failure();
+  } else {
+    semanticsContext.messages().Annex(std::move(parsing.messages()));
   }
 
   // run semantics