[flang][nfc] Tweak the FrontendAction class

[awarzynski]

Summary

This patch refactors the FrontendAction class. It merely moves code
around so that re-using it is easier. No new functionality is
introduced.

1. Three new member methods are introduced: RunPrescan, RunParse,

RunSema.

2. The following free functions are re-implemented as member methods:
   • reportFatalSemanticErrors
   • reportFatalScanningErrors
   • reportFatalParsingErrors
   • reportFatalErrors

reportFatalSemanticErrors is updated to resemble the other error
reporting functions and to make the API more consistent.

3. The BeginSourceFileAction methods are simplified and the unused

input argument is deleted.

[ashermancinelli]

flang/lib/Frontend/FrontendAction.cpp

159

[nit] Would it be too invasive to expand this name in this patch, eg RunSemanticAction or something else more verbose?

[awarzynski]

flang/lib/Frontend/FrontendAction.cpp

159

RunSema is a poor name, thanks for complaining :)

I'd rather avoid using Action here as this method does not correspond to a particular FrontendAction. Instead, it will be used by multiple specialisations of that class. Let me rename it as RunSemanticChecks.

[awarzynski]

Rename RunSema as RunSemanticChecks

[ashermancinelli]

flang/lib/Frontend/FrontendAction.cpp

159

Looks great!

flang/include/flang/Frontend/FrontendAction.h

protected:
   ///
   /// By default it returns true if a compiler error occurred.
   virtual bool ShouldEraseOutputFiles();
 
   /// Callback at the start of processing a single input.
   ///
   /// \return True on success; on failure ExecutionAction() and
   /// EndSourceFileAction() will not be called.
-  virtual bool BeginSourceFileAction(CompilerInstance &ci) { return true; }
+  virtual bool BeginSourceFileAction() { return true; }
 
   /// @}
 
 public:
   FrontendAction() : instance_(nullptr) {}
   virtual ~FrontendAction() = default;
 
   /// @name Compiler Instance Access
public:
   bool BeginSourceFile(CompilerInstance &ci, const FrontendInputFile &input);
 
   /// Run the action.
   llvm::Error Execute();
 
   /// Perform any per-file post processing, deallocate per-file
   /// objects, and run statistics and output file cleanup code.
   void EndSourceFile();
+
+  /// @}
+protected:
+  // Prescan the current input file. Return False if fatal errors are reported,
+  // True otherwise.
+  bool RunPrescan();
+  // Parse the current input file. Return False if fatal errors are reported,
+  // True otherwise.
+  bool RunParse();
+  // Run semantic checks for the current input file. Return False if fatal
+  // errors are reported, True otherwise.
+  bool RunSemanticChecks();
+
+  // Report fatal semantic errors. Return True if present, false otherwise.
+  bool reportFatalSemanticErrors();
+
+  // Report fatal scanning errors. Return True if present, false otherwise.
+  inline bool reportFatalScanningErrors() {
+    return reportFatalErrors("Could not scan %0");
+  }
+
+  // Report fatal parsing errors. Return True if present, false otherwise
+  inline bool reportFatalParsingErrors() {
+    return reportFatalErrors("Could not parse %0");
+  }
+
+private:
+  template <unsigned N> bool reportFatalErrors(const char (&message)[N]);
 };
 
 } // namespace Fortran::frontend
 
 #endif // LLVM_FLANG_FRONTEND_FRONTENDACTION_H

flang/include/flang/Frontend/FrontendActions.h

class InitOnlyAction : public FrontendAction {
   void ExecuteAction() override;
 };
 
 //===----------------------------------------------------------------------===//
 // Prescan Actions
 //===----------------------------------------------------------------------===//
 class PrescanAction : public FrontendAction {
   void ExecuteAction() override = 0;
-  bool BeginSourceFileAction(CompilerInstance &ci) override;
+  bool BeginSourceFileAction() override;
 };
 
 class PrintPreprocessedAction : public PrescanAction {
   void ExecuteAction() override;
 };
 
 class DebugDumpProvenanceAction : public PrescanAction {
   void ExecuteAction() override;
 };
 
 class DebugDumpParsingLogAction : public PrescanAction {
   void ExecuteAction() override;
 };
 
 class DebugMeasureParseTreeAction : public PrescanAction {
   void ExecuteAction() override;
 };
 
 //===----------------------------------------------------------------------===//
 // PrescanAndParse Actions
 //===----------------------------------------------------------------------===//
 class PrescanAndParseAction : public FrontendAction {
   void ExecuteAction() override = 0;
-  bool BeginSourceFileAction(CompilerInstance &ci) override;
+  bool BeginSourceFileAction() override;
 };
 
 class DebugUnparseNoSemaAction : public PrescanAndParseAction {
   void ExecuteAction() override;
 };
 
 class DebugDumpParseTreeNoSemaAction : public PrescanAndParseAction {
   void ExecuteAction() override;
 };
 
 //===----------------------------------------------------------------------===//
 // PrescanAndSema Actions
 //===----------------------------------------------------------------------===//
 class PrescanAndSemaAction : public FrontendAction {
 
   void ExecuteAction() override = 0;
-  bool BeginSourceFileAction(CompilerInstance &ci) override;
+  bool BeginSourceFileAction() override;
 };
 
 class DebugUnparseWithSymbolsAction : public PrescanAndSemaAction {
   void ExecuteAction() override;
 };
 
 class DebugUnparseAction : public PrescanAndSemaAction {
   void ExecuteAction() override;

flang/lib/Frontend/FrontendAction.cpp

bool FrontendAction::BeginSourceFile(
   }
 
   // Decide between fixed and free form (if the user didn't express any
   // preference, use the file extension to decide)
   if (invoc.frontendOpts().fortranForm == FortranForm::Unknown) {
     invoc.fortranOpts().isFixedForm = currentInput().IsFixedForm();
   }
 
-  if (!BeginSourceFileAction(ci)) {
+  if (!BeginSourceFileAction()) {
     BeginSourceFileCleanUp(*this, ci);
     return false;
   }
 
   return true;
 }
 
 bool FrontendAction::ShouldEraseOutputFiles() {
void FrontendAction::EndSourceFile() {
 
   // Cleanup the output streams, and erase the output files if instructed by the
   // FrontendAction.
   ci.ClearOutputFiles(/*EraseFiles=*/ShouldEraseOutputFiles());
 
   set_instance(nullptr);
   set_currentInput(FrontendInputFile());
 }
+
+bool FrontendAction::RunPrescan() {
+  CompilerInstance &ci = this->instance();
+  std::string currentInputPath{GetCurrentFileOrBufferName()};
+  Fortran::parser::Options parserOptions = ci.invocation().fortranOpts();
+
+  if (ci.invocation().frontendOpts().fortranForm == FortranForm::Unknown) {
+    // Switch between fixed and free form format based on the input file
+    // extension.
+    //
+    // Ideally we should have all Fortran options set before entering this
+    // method (i.e. before processing any specific input files). However, we
+    // can't decide between fixed and free form based on the file extension
+    // earlier than this.
+    parserOptions.isFixedForm = currentInput().IsFixedForm();
+  }
+
+  // Prescan. In case of failure, report and return.
+  ci.parsing().Prescan(currentInputPath, parserOptions);
+
+  return !reportFatalScanningErrors();
+}
+
+bool FrontendAction::RunParse() {
+  CompilerInstance &ci = this->instance();
+
+  // Parse. In case of failure, report and return.
+  ci.parsing().Parse(llvm::outs());
+
+  if (reportFatalParsingErrors()) {
+    return false;
+  }
+
+  // Report the diagnostics from parsing
+  ci.parsing().messages().Emit(llvm::errs(), ci.allCookedSources());
+
+  return true;
+}
+
+bool FrontendAction::RunSemanticChecks() {
+  CompilerInstance &ci = this->instance();
+  std::optional<parser::Program> &parseTree{ci.parsing().parseTree()};
+  assert(parseTree && "Cannot run semantic checks without a parse tree!");
+
+  // Prepare semantics
+  ci.setSemantics(std::make_unique<Fortran::semantics::Semantics>(
+      ci.invocation().semanticsContext(), *parseTree,
+      ci.invocation().debugModuleDir()));
+  auto &semantics = ci.semantics();
+
+  // Run semantic checks
+  semantics.Perform();
+
+  if (reportFatalSemanticErrors()) {
+    return false;
+  }
+
+  // Report the diagnostics from the semantic checks
+  semantics.EmitMessages(ci.semaOutputStream());
+
+  return true;
+}
+
+template <unsigned N>
+bool FrontendAction::reportFatalErrors(const char (&message)[N]) {
+  if (!instance_->parsing().messages().empty() &&
+      (instance_->invocation().warnAsErr() ||
+          instance_->parsing().messages().AnyFatalError())) {
+    const unsigned diagID = instance_->diagnostics().getCustomDiagID(
+        clang::DiagnosticsEngine::Error, message);
+    instance_->diagnostics().Report(diagID) << GetCurrentFileOrBufferName();
+    instance_->parsing().messages().Emit(
+        llvm::errs(), instance_->allCookedSources());
+    return true;
+  }
+  return false;
+}
+
+bool FrontendAction::reportFatalSemanticErrors() {
+  auto &diags = instance_->diagnostics();
+  auto &sema = instance_->semantics();
+
+  if (instance_->semantics().AnyFatalError()) {
+    unsigned DiagID = diags.getCustomDiagID(
+        clang::DiagnosticsEngine::Error, "Semantic errors in %0");
+    diags.Report(DiagID) << GetCurrentFileOrBufferName();
+    sema.EmitMessages(instance_->semaOutputStream());
+
+    return true;
+  }
+
+  return false;
+}

flang/lib/Frontend/FrontendActions.cpp

 #include "flang/Semantics/unparse-with-symbols.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/ErrorHandling.h"
 #include <clang/Basic/Diagnostic.h>
 #include <memory>
 
 using namespace Fortran::frontend;
 
-/// Report fatal semantic errors if present.
-///
-/// \param semantics The semantics instance
-/// \param diags The diagnostics engine instance
-/// \param bufferName The file or buffer name
-///
-/// \return True if fatal semantic errors are present, false if not
-bool reportFatalSemanticErrors(const Fortran::semantics::Semantics &semantics,
-    clang::DiagnosticsEngine &diags, const llvm::StringRef &bufferName) {
-  if (semantics.AnyFatalError()) {
-    unsigned DiagID = diags.getCustomDiagID(
-        clang::DiagnosticsEngine::Error, "Semantic errors in %0");
-    diags.Report(DiagID) << bufferName;
-    return true;
-  }
-  return false;
-}
-
-template <unsigned N>
-static bool reportFatalErrors(
-    const FrontendAction *act, const char (&message)[N]) {
-  CompilerInstance &ci = act->instance();
-  if (!ci.parsing().messages().empty() &&
-      (ci.invocation().warnAsErr() ||
-          ci.parsing().messages().AnyFatalError())) {
-    const unsigned diagID = ci.diagnostics().getCustomDiagID(
-        clang::DiagnosticsEngine::Error, message);
-    ci.diagnostics().Report(diagID) << act->GetCurrentFileOrBufferName();
-    ci.parsing().messages().Emit(llvm::errs(), ci.allCookedSources());
-    return true;
-  }
-  return false;
-}
-
-inline bool reportFatalScanningErrors(const FrontendAction *act) {
-  return reportFatalErrors(act, "Could not scan %0");
-}
-
-inline bool reportFatalParsingErrors(const FrontendAction *act) {
-  return reportFatalErrors(act, "Could not parse %0");
-}
-
-bool PrescanAction::BeginSourceFileAction(CompilerInstance &c1) {
-  CompilerInstance &ci = this->instance();
-  std::string currentInputPath{GetCurrentFileOrBufferName()};
-  Fortran::parser::Options parserOptions = ci.invocation().fortranOpts();
-
-  // Prescan. In case of failure, report and return.
-  ci.parsing().Prescan(currentInputPath, parserOptions);
-
-  return !reportFatalScanningErrors(this);
-}
-
-bool PrescanAndParseAction::BeginSourceFileAction(CompilerInstance &c1) {
-  CompilerInstance &ci = this->instance();
-
-  std::string currentInputPath{GetCurrentFileOrBufferName()};
-
-  Fortran::parser::Options parserOptions = ci.invocation().fortranOpts();
-
-  if (ci.invocation().frontendOpts().fortranForm == FortranForm::Unknown) {
-    // Switch between fixed and free form format based on the input file
-    // extension.
-    //
-    // Ideally we should have all Fortran options set before entering this
-    // method (i.e. before processing any specific input files). However, we
-    // can't decide between fixed and free form based on the file extension
-    // earlier than this.
-    parserOptions.isFixedForm = currentInput().IsFixedForm();
-  }
-
-  // Prescan. In case of failure, report and return.
-  ci.parsing().Prescan(currentInputPath, parserOptions);
-
-  if (reportFatalScanningErrors(this))
-    return false;
-
-  // Parse. In case of failure, report and return.
-  ci.parsing().Parse(llvm::outs());
-
-  if (reportFatalParsingErrors(this))
-    return false;
-
-  // Report the diagnostics from parsing
-  ci.parsing().messages().Emit(llvm::errs(), ci.allCookedSources());
+//===----------------------------------------------------------------------===//
+// Custom BeginSourceFileAction
+//===----------------------------------------------------------------------===//
+bool PrescanAction::BeginSourceFileAction() { return RunPrescan(); }
 
-  return true;
+bool PrescanAndParseAction::BeginSourceFileAction() {
+  return RunPrescan() && RunParse();
 }
 
-bool PrescanAndSemaAction::BeginSourceFileAction(CompilerInstance &c1) {
-  CompilerInstance &ci = this->instance();
-  std::string currentInputPath{GetCurrentFileOrBufferName()};
-  Fortran::parser::Options parserOptions = ci.invocation().fortranOpts();
-
-  // Prescan. In case of failure, report and return.
-  ci.parsing().Prescan(currentInputPath, parserOptions);
-
-  if (reportFatalScanningErrors(this))
-    return false;
-
-  // Parse. In case of failure, report and return.
-  ci.parsing().Parse(llvm::outs());
-
-  if (reportFatalParsingErrors(this))
-    return false;
-
-  // Report the diagnostics from parsing
-  ci.parsing().messages().Emit(llvm::errs(), ci.allCookedSources());
-
-  auto &parseTree{*ci.parsing().parseTree()};
-
-  // Prepare semantics
-  ci.setSemantics(std::make_unique<Fortran::semantics::Semantics>(
-      ci.invocation().semanticsContext(), parseTree,
-      ci.invocation().debugModuleDir()));
-  auto &semantics = ci.semantics();
-
-  // Run semantic checks
-  semantics.Perform();
-
-  // Report the diagnostics from the semantic checks
-  semantics.EmitMessages(ci.semaOutputStream());
-
-  return true;
+bool PrescanAndSemaAction::BeginSourceFileAction() {
+  return RunPrescan() & RunParse() && RunSemanticChecks();
 }
 
+//===----------------------------------------------------------------------===//
+// Custom ExecuteAction
+//===----------------------------------------------------------------------===//
 void InputOutputTestAction::ExecuteAction() {
   CompilerInstance &ci = instance();
 
   // Create a stream for errors
   std::string buf;
   llvm::raw_string_ostream error_stream{buf};
 
   // Read the input file
void PrintPreprocessedAction::ExecuteAction() {
   }
 }
 
 void DebugDumpProvenanceAction::ExecuteAction() {
   this->instance().parsing().DumpProvenance(llvm::outs());
 }
 
 void ParseSyntaxOnlyAction::ExecuteAction() {
-  CompilerInstance &ci = this->instance();
-
-  reportFatalSemanticErrors(
-      ci.semantics(), ci.diagnostics(), GetCurrentFileOrBufferName());
 }
 
 void DebugUnparseNoSemaAction::ExecuteAction() {
   auto &invoc = this->instance().invocation();
   auto &parseTree{instance().parsing().parseTree()};
 
   // TODO: Options should come from CompilerInvocation
   Unparse(llvm::outs(), *parseTree,
void DebugUnparseAction::ExecuteAction() {
   // TODO: Options should come from CompilerInvocation
   Unparse(*os, *parseTree,
       /*encoding=*/Fortran::parser::Encoding::UTF_8,
       /*capitalizeKeywords=*/true, /*backslashEscapes=*/false,
       /*preStatement=*/nullptr,
       invoc.useAnalyzedObjectsForUnparse() ? &invoc.asFortran() : nullptr);
 
   // Report fatal semantic errors
-  reportFatalSemanticErrors(ci.semantics(), this->instance().diagnostics(),
-      GetCurrentFileOrBufferName());
+  reportFatalSemanticErrors();
 }
 
 void DebugUnparseWithSymbolsAction::ExecuteAction() {
-  CompilerInstance &ci = this->instance();
   auto &parseTree{*instance().parsing().parseTree()};
 
   Fortran::semantics::UnparseWithSymbols(
       llvm::outs(), parseTree, /*encoding=*/Fortran::parser::Encoding::UTF_8);
 
   // Report fatal semantic errors
-  reportFatalSemanticErrors(
-      ci.semantics(), ci.diagnostics(), GetCurrentFileOrBufferName());
+  reportFatalSemanticErrors();
 }
 
 void DebugDumpSymbolsAction::ExecuteAction() {
   CompilerInstance &ci = this->instance();
   auto &semantics = ci.semantics();
 
   auto tables{Fortran::semantics::BuildRuntimeDerivedTypeTables(
       instance().invocation().semanticsContext())};
   // The runtime derived type information table builder may find and report
   // semantic errors. So it is important that we report them _after_
   // BuildRuntimeDerivedTypeTables is run.
-  reportFatalSemanticErrors(
-      semantics, this->instance().diagnostics(), GetCurrentFileOrBufferName());
+  reportFatalSemanticErrors();
 
   if (!tables.schemata) {
     unsigned DiagID =
         ci.diagnostics().getCustomDiagID(clang::DiagnosticsEngine::Error,
             "could not find module file for __fortran_type_info");
     ci.diagnostics().Report(DiagID);
     llvm::errs() << "\n";
   }
Fortran::parser::DumpTree(
       llvm::outs(), parseTree, &ci.invocation().asFortran());
 
   auto &semantics = ci.semantics();
   auto tables{Fortran::semantics::BuildRuntimeDerivedTypeTables(
       instance().invocation().semanticsContext())};
   // The runtime derived type information table builder may find and report
   // semantic errors. So it is important that we report them _after_
   // BuildRuntimeDerivedTypeTables is run.
-  reportFatalSemanticErrors(
-      semantics, this->instance().diagnostics(), GetCurrentFileOrBufferName());
+  reportFatalSemanticErrors();
 
   if (!tables.schemata) {
     unsigned DiagID =
         ci.diagnostics().getCustomDiagID(clang::DiagnosticsEngine::Error,
             "could not find module file for __fortran_type_info");
     ci.diagnostics().Report(DiagID);
     llvm::errs() << "\n";
   }
void DebugDumpParseTreeNoSemaAction::ExecuteAction() {
   auto &parseTree{instance().parsing().parseTree()};
 
   // Dump parse tree
   Fortran::parser::DumpTree(
       llvm::outs(), parseTree, &this->instance().invocation().asFortran());
 }
 
 void DebugDumpParseTreeAction::ExecuteAction() {
-  CompilerInstance &ci = this->instance();
   auto &parseTree{instance().parsing().parseTree()};
 
   // Dump parse tree
   Fortran::parser::DumpTree(
       llvm::outs(), parseTree, &this->instance().invocation().asFortran());
 
   // Report fatal semantic errors
-  reportFatalSemanticErrors(
-      ci.semantics(), ci.diagnostics(), GetCurrentFileOrBufferName());
+  reportFatalSemanticErrors();
 }
 
 void DebugMeasureParseTreeAction::ExecuteAction() {
   CompilerInstance &ci = this->instance();
 
   // Parse. In case of failure, report and return.
   ci.parsing().Parse(llvm::outs());
 
void DebugMeasureParseTreeAction::ExecuteAction() {
   llvm::outs() << "Parse tree comprises " << visitor.objects
                << " objects and occupies " << visitor.bytes
                << " total bytes.\n";
 }
 
 void DebugPreFIRTreeAction::ExecuteAction() {
   CompilerInstance &ci = this->instance();
   // Report and exit if fatal semantic errors are present
-  if (reportFatalSemanticErrors(
-          ci.semantics(), ci.diagnostics(), GetCurrentFileOrBufferName())) {
+  if (reportFatalSemanticErrors()) {
     return;
   }
 
   auto &parseTree{*ci.parsing().parseTree()};
 
   // Dump pre-FIR tree
   if (auto ast{Fortran::lower::createPFT(
           parseTree, ci.invocation().semanticsContext())}) {
void DebugDumpParsingLogAction::ExecuteAction() {
   ci.parsing().Parse(llvm::errs());
   ci.parsing().DumpParsingLog(llvm::outs());
 }
 
 void GetDefinitionAction::ExecuteAction() {
   CompilerInstance &ci = this->instance();
 
   // Report and exit if fatal semantic errors are present
-  if (reportFatalSemanticErrors(
-          ci.semantics(), ci.diagnostics(), GetCurrentFileOrBufferName()))
+  if (reportFatalSemanticErrors()) {
     return;
+  }
 
   parser::AllCookedSources &cs = ci.allCookedSources();
   unsigned diagID = ci.diagnostics().getCustomDiagID(
       clang::DiagnosticsEngine::Error, "Symbol not found");
 
   auto gdv = ci.invocation().frontendOpts().getDefVals;
   auto charBlock{cs.GetCharBlockFromLineAndColumns(
       gdv.line, gdv.startColumn, gdv.endColumn)};
llvm::outs() << symbol->name().ToString() << ": "
                << sourceInfo->first.line << ", " << sourceInfo->first.column
                << "-" << sourceInfo->second.column << "\n";
 }
 
 void GetSymbolsSourcesAction::ExecuteAction() {
   CompilerInstance &ci = this->instance();
 
   // Report and exit if fatal semantic errors are present
-  if (reportFatalSemanticErrors(
-          ci.semantics(), ci.diagnostics(), GetCurrentFileOrBufferName()))
+  if (reportFatalSemanticErrors()) {
     return;
+  }
 
   ci.semantics().DumpSymbolsSources(llvm::outs());
 }
 
 void EmitObjAction::ExecuteAction() {
   CompilerInstance &ci = this->instance();
   unsigned DiagID = ci.diagnostics().getCustomDiagID(
       clang::DiagnosticsEngine::Error, "code-generation is not available yet");