[flang][driver] Add checks for errors from `Prescan` and `Parse`

[awarzynski]

Summary

If either Prescan or Parse generate any fatal errors, the new driver
will:

• report it (i.e. issue an error diagnostic)
• exit early
• return non-zero exit code

This behaviour is consistent with f18 (i.e. the old driver).

[sameeranjoshi]

Mostly nit comments.
Thanks and LGMT.

flang/lib/Frontend/FrontendAction.cpp

59

nit - would be better to use const as much as possible if there are not modifications to the variables.

point 13
https://github.com/llvm/llvm-project/blob/master/flang/docs/C++style.md#c-language

60

nit: - Should the error messages start with a capital letter?
https://github.com/llvm/llvm-project/blob/master/flang/docs/C++style.md#error-messages

63

nit - ci ?

[sscalpone]

I can see how these messages can be useful for a compiler developer; however, perhaps these messages are redundant for end users because other error messages have been issued.

[awarzynski]

Thanks @sameeranjoshi !

flang/lib/Frontend/FrontendAction.cpp

60

Good point. The current implementation is consistent with f18: https://github.com/llvm/llvm-project/blob/main/flang/tools/f18/f18.cpp#L212 and that's what I've been prioritizing. I'm not sure whether f18 is intentionally inconsistent with the coding guidelines or not.

I suggest that these messages are updated in a separate patch for both f18 and flang-new. Since it's an NFC, I pushed it without a separate review (https://github.com/llvm/llvm-project/commit/fa1e543e0b8c625bf2625598d9a16c484e349884). Thanks for pointing this out!

[awarzynski]

In D93712#2481647, @sscalpone wrote:

I can see how these messages can be useful for a compiler developer; however, perhaps these messages are redundant for end users because other error messages have been issued.

Thank you for the suggestion! I actually pushed this change pretty much at the point when you were sending this. Obviously we can refine this.

For now I'd like to prioritise consistency with f18. Once flang-new is ready to replace f18, we could focus on refining the user experience. Also, when end-users report erros to developers, it's good to have a message that allows developers narrow the error down. So I still think that it's worthwhile to have specialized error messages (e.g. could not parse vs could not scan) in the driver. Lastly, wouldn't the frontend driver be considered more of a _developer_ rather than _end-user_ tool?

flang/lib/Frontend/FrontendAction.cpp

bool FrontendAction::BeginSourceFile(
   return true;
 }
 
 bool FrontendAction::ShouldEraseOutputFiles() {
   return instance().diagnostics().hasErrorOccurred();
 }
 
 llvm::Error FrontendAction::Execute() {
+  CompilerInstance &ci = this->instance();
+
   std::string currentInputPath{GetCurrentFileOrBufferName()};
 
   Fortran::parser::Options parserOptions =
       this->instance().invocation().fortranOpts();
 
-  this->instance().parsing().Prescan(currentInputPath, parserOptions);
+  // Prescan. In case of failure, report and return.
+  ci.parsing().Prescan(currentInputPath, parserOptions);
+
+  if (ci.parsing().messages().AnyFatalError()) {
+    const unsigned diagID = ci.diagnostics().getCustomDiagID(
+        clang::DiagnosticsEngine::Error, "could not scan %0");
+    ci.diagnostics().Report(diagID) << GetCurrentFileOrBufferName();
+    ci.parsing().messages().Emit(llvm::errs(), ci.allCookedSources());
+
+    return llvm::Error::success();
+  }
 
   ExecuteAction();
 
   return llvm::Error::success();
 }
 
 void FrontendAction::EndSourceFile() {
   CompilerInstance &ci = instance();
 
   // Cleanup the output streams, and erase the output files if instructed by the
   // FrontendAction.
   ci.ClearOutputFiles(/*EraseFiles=*/ShouldEraseOutputFiles());
 
   set_instance(nullptr);
   set_currentInput(FrontendInputFile());
 }

flang/lib/Frontend/FrontendActions.cpp

 
 void ParseSyntaxOnlyAction::ExecuteAction() {
   CompilerInstance &ci = this->instance();
 
   // TODO: These should be specifiable by users. For now just use the defaults.
   common::LanguageFeatureControl features;
   Fortran::common::IntrinsicTypeDefaultKinds defaultKinds;
 
-  // Parse
+  // Parse. In case of failure, report and return.
   ci.parsing().Parse(llvm::outs());
+
+  if (ci.parsing().messages().AnyFatalError()) {
+    unsigned diagID = ci.diagnostics().getCustomDiagID(
+        clang::DiagnosticsEngine::Error, "could not parse %0");
+    ci.diagnostics().Report(diagID) << GetCurrentFileOrBufferName();
+
+    ci.parsing().messages().Emit(
+        llvm::errs(), this->instance().allCookedSources());
+    return;
+  }
+
   auto &parseTree{*ci.parsing().parseTree()};
 
   // Prepare semantics
   Fortran::semantics::SemanticsContext semanticsContext{
       defaultKinds, features, ci.allCookedSources()};
   Fortran::semantics::Semantics semantics{
       semanticsContext, parseTree, ci.parsing().cooked().AsCharBlock()};
 

flang/test/Flang-Driver/parse-error.f95

+! RUN: not %flang-new -fc1 -fsyntax-only %s 2>&1 | FileCheck %s --check-prefix=ERROR
+! RUN: not %f18 -parse-only %s 2>&1 | FileCheck %s --check-prefix=ERROR
+
+! REQUIRES: new-flang-driver
+
+! ERROR: could not parse {{.*}}parse-error.f95
+
+"This file will not parse"

flang/test/Flang-Driver/scanning-error.f95

+! RUN: not %flang-new -fc1 -E %s 2>&1 | FileCheck %s --check-prefix=ERROR
+! RUN: not %f18 -E %s 2>&1 | FileCheck %s --check-prefix=ERROR
+
+! REQUIRES: new-flang-driver
+
+! ERROR: could not scan {{.*}}scanning-error.f95
+
+#NOT-PP-DIRECTIVE