[flang][driver] Add `-fdebug-dump-parsing-log`

[awarzynski]

Summary

Adds -fdebug-dump-parsing-log in the new driver. This option is
semantically identical to -fdebug-instrumented-parse in f18 (the
former is added as an alias in f18).

As dumping the parsing log makes only sense for instrumented parses, we
set Fortran::parser::Options::instrumentedParse to True when
-fdebug-dump-parsing-log is used. This is consistent with f18. To
facilate tweaking the frontend based on the action being run,
setUpFrontendBasedOnAction in CompilerInvocation.cpp is introduced.

[AMDChirag]

flang/lib/Frontend/CompilerInvocation.cpp

88–95

Will this function be extended in the future?
If not, an entirely separate function for a couple statements seems rather overkill.

flang/lib/Frontend/FrontendActions.cpp

313–316

NIT: what's the point of these comments?
They are pretty much exactly the same as the function names.

[awarzynski]

Remove spurious comments

[awarzynski]

flang/lib/Frontend/CompilerInvocation.cpp

88–95

Will this function be extended in the future?

Yes.

Ideally we'd want _features_ and _actions_ to be orthogonal and to be controlled by dedicated flags. In this patch, we are adding an _action_ flag that toggles a _feature_ option. I think that that's a bit counter-intuitive, but not uncommon or unavoidable long-term.

Instead of adding comments, I prefer to introduce a dedicated method for this logic. It will make it easier for us to keep it in one place when new options like this are added. Also, ParseFrontendArgs is already quite long and is only going to get longer.

Having said all that, we may decide in the future that there's a better way to split this logic. For now I mostly want to avoid extending ParseFrontendArgs too much.

flang/lib/Frontend/FrontendActions.cpp

313–316

Good point, thank you!

[AMDChirag]

flang/lib/Frontend/CompilerInvocation.cpp

88–95

Fair, thank you for explaining!

In such case, shouldn't the function call setUpFrontendBasedOnAction(opts); be placed after all the fields of the opts variable have been set?

[awarzynski]

Move the call to setUpFrontendBasedOnAction after all options have been set

[awarzynski]

flang/lib/Frontend/CompilerInvocation.cpp

88–95

Thanks for the suggestion, updated!

[AMDChirag]

LGTM.
I'll let someone else review the patch if required.

[arnamoy10]

LGTM

[ashermancinelli]

It is unintuitive to me that we are mixing feature and action flags, however I don't have a better suggestion. LGTM.

[awarzynski]

Generalise the test so that it's not architecture specific

I've just realised that the test was failing in Phabricator's CI. It turns out it's an X86-only failure (i.e. the test works fine on my AArch64 dev machine). This boils down to the order in which messages are printed. As the logs are unordered, one gets slightly different results on every platform. I extracted _one log_ (i.e. a block of N lines that's part of one log message) and used that for testing instead. Verified on both X86 and AArch64.

clang/include/clang/Driver/Options.td

 def fdebug_unparse_with_symbols : Flag<["-"], "fdebug-unparse-with-symbols">, Group<Action_Group>,
   HelpText<"Unparse and stop.">;
 def fdebug_dump_symbols : Flag<["-"], "fdebug-dump-symbols">, Group<Action_Group>,
   HelpText<"Dump symbols after the semantic analysis">;
 def fdebug_dump_parse_tree : Flag<["-"], "fdebug-dump-parse-tree">, Group<Action_Group>,
   HelpText<"Dump the parse tree">;
 def fdebug_dump_provenance : Flag<["-"], "fdebug-dump-provenance">, Group<Action_Group>,
   HelpText<"Dump provenance">;
+def fdebug_dump_parsing_log : Flag<["-"], "fdebug-dump-parsing-log">, Group<Action_Group>,
+  HelpText<"Run instrumented parse and dump the parsing log">;
 def fdebug_measure_parse_tree : Flag<["-"], "fdebug-measure-parse-tree">, Group<Action_Group>,
   HelpText<"Measure the parse tree">;
 def fdebug_pre_fir_tree : Flag<["-"], "fdebug-pre-fir-tree">, Group<Action_Group>,
   HelpText<"Dump the pre-FIR tree">;
 
 }
 
 //===----------------------------------------------------------------------===//

flang/include/flang/Frontend/FrontendActions.h

 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;
 };
 
 //===----------------------------------------------------------------------===//
 // PrescanAndSema Actions
 //===----------------------------------------------------------------------===//
 class PrescanAndSemaAction : public FrontendAction {

flang/include/flang/Frontend/FrontendOptions.h

enum ActionKind {
   DebugDumpSymbols,
 
   /// Parse, run semantics and then output the parse tree
   DebugDumpParseTree,
 
   /// Dump provenance
   DebugDumpProvenance,
 
+  /// Parse then output the parsing log
+  DebugDumpParsingLog,
+
   /// Parse then output the number of objects in the parse tree and the overall
   /// size
   DebugMeasureParseTree,
 
   /// Parse, run semantics and then output the pre-FIR tree
   DebugPreFIRTree
 
   /// TODO: RunPreprocessor, EmitLLVM, EmitLLVMOnly,

 class FrontendOptions {
 public:
   /// Show the -help text.
   unsigned showHelp_ : 1;
 
   /// Show the -version text.
   unsigned showVersion_ : 1;
 
+  /// Instrument the parse to get a more verbose log
+  unsigned instrumentedParse_ : 1;
+
   /// The input files and their types.
   std::vector<FrontendInputFile> inputs_;
 
   /// The output file, if any.
   std::string outputFile_;
 
   /// The frontend action to perform.
   frontend::ActionKind programAction_;

flang/lib/Frontend/CompilerInvocation.cpp

 
 bool Fortran::frontend::ParseDiagnosticArgs(clang::DiagnosticOptions &opts,
     llvm::opt::ArgList &args, bool defaultDiagColor) {
   opts.ShowColors = parseShowColorsArgs(args, defaultDiagColor);
 
   return true;
 }
 
+// Tweak the frontend configuration based on the frontend action
+static void setUpFrontendBasedOnAction(FrontendOptions &opts) {
+  assert(opts.programAction_ != Fortran::frontend::InvalidAction &&
+      "Fortran frontend action not set!");
+
+  if (opts.programAction_ == DebugDumpParsingLog)
+    opts.instrumentedParse_ = true;
+}
+
 static InputKind ParseFrontendArgs(FrontendOptions &opts,
     llvm::opt::ArgList &args, clang::DiagnosticsEngine &diags) {
 
   // By default the frontend driver creates a ParseSyntaxOnly action.
   opts.programAction_ = ParseSyntaxOnly;
 
   // Identify the action (i.e. opts.ProgramAction)
   if (const llvm::opt::Arg *a =
case clang::driver::options::OPT_fdebug_dump_symbols:
       opts.programAction_ = DebugDumpSymbols;
       break;
     case clang::driver::options::OPT_fdebug_dump_parse_tree:
       opts.programAction_ = DebugDumpParseTree;
       break;
     case clang::driver::options::OPT_fdebug_dump_provenance:
       opts.programAction_ = DebugDumpProvenance;
       break;
+    case clang::driver::options::OPT_fdebug_dump_parsing_log:
+      opts.programAction_ = DebugDumpParsingLog;
+      break;
     case clang::driver::options::OPT_fdebug_measure_parse_tree:
       opts.programAction_ = DebugMeasureParseTree;
       break;
     case clang::driver::options::OPT_fdebug_pre_fir_tree:
       opts.programAction_ = DebugPreFIRTree;
       break;
 
       // TODO:
if (argValue == "utf-8") {
       opts.encoding_ = Fortran::parser::Encoding::UTF_8;
     } else if (argValue == "latin-1") {
       opts.encoding_ = Fortran::parser::Encoding::LATIN_1;
     } else {
       diags.Report(clang::diag::err_drv_invalid_value)
           << arg->getAsString(args) << argValue;
     }
   }
+
+  setUpFrontendBasedOnAction(opts);
+
   return dashX;
 }
 
 /// Parses all preprocessor input arguments and populates the preprocessor
 /// options accordingly.
 ///
 /// \param [in] opts The preprocessor options instance
 /// \param [out] args The list of input arguments
fortranOptions.searchDirectories.insert(
       fortranOptions.searchDirectories.end(),
       preprocessorOptions.searchDirectoriesFromDashI.begin(),
       preprocessorOptions.searchDirectoriesFromDashI.end());
 
   // Add the directory supplied through -J/-module-dir to the list of search
   // directories
   if (moduleDirJ.compare(".") != 0)
     fortranOptions.searchDirectories.emplace_back(moduleDirJ);
+
+  if (frontendOptions.instrumentedParse_)
+    fortranOptions.instrumentedParse = true;
 }
 
 void CompilerInvocation::setSemanticsOpts(
     Fortran::parser::AllCookedSources &allCookedSources) {
   const auto &fortranOptions = fortranOpts();
 
   semanticsContext_ = std::make_unique<semantics::SemanticsContext>(
       defaultKinds(), fortranOptions.features, allCookedSources);
 
   auto &moduleDirJ = moduleDir();
   semanticsContext_->set_moduleDirectory(moduleDirJ)
       .set_searchDirectories(fortranOptions.searchDirectories);
 }

flang/lib/Frontend/FrontendActions.cpp

if (auto ast{Fortran::lower::createPFT(
     Fortran::lower::dumpPFT(llvm::outs(), *ast);
   } else {
     unsigned diagID = ci.diagnostics().getCustomDiagID(
         clang::DiagnosticsEngine::Error, "Pre FIR Tree is NULL.");
     ci.diagnostics().Report(diagID);
   }
 }
 
+void DebugDumpParsingLogAction::ExecuteAction() {
+  CompilerInstance &ci = this->instance();
+
+  ci.parsing().Parse(llvm::errs());
+  ci.parsing().DumpParsingLog(llvm::outs());
+}
+
 void EmitObjAction::ExecuteAction() {
   CompilerInstance &ci = this->instance();
   unsigned DiagID = ci.diagnostics().getCustomDiagID(
       clang::DiagnosticsEngine::Error, "code-generation is not available yet");
   ci.diagnostics().Report(DiagID);
 }

flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp

case DebugDumpSymbols:
     return std::make_unique<DebugDumpSymbolsAction>();
     break;
   case DebugDumpParseTree:
     return std::make_unique<DebugDumpParseTreeAction>();
     break;
   case DebugDumpProvenance:
     return std::make_unique<DebugDumpProvenanceAction>();
     break;
+  case DebugDumpParsingLog:
+    return std::make_unique<DebugDumpParsingLogAction>();
+    break;
   case DebugMeasureParseTree:
     return std::make_unique<DebugMeasureParseTreeAction>();
     break;
   case DebugPreFIRTree:
     return std::make_unique<DebugPreFIRTreeAction>();
     break;
   default:
     break;

flang/test/Driver/debug-parsing-log.f90

+! RUN: %flang_fc1 -fdebug-dump-parsing-log %s  2>&1 | FileCheck %s
+
+!-----------------
+! EXPECTED OUTPUT
+!-----------------
+! Below are just few lines extracted from the dump. The actual output is much _much_ bigger.
+
+! CHECK: {{.*}}/debug-parsing-log.f90:31:1: IMPLICIT statement
+! CHECK-NEXT:  END PROGRAM
+! CHECK-NEXT:  ^
+! CHECK-NEXT:  fail 3
+! CHECK-NEXT: {{.*}}/debug-parsing-log.f90:31:1: error: expected 'IMPLICIT NONE'
+! CHECK-NEXT:   END PROGRAM
+! CHECK-NEXT:   ^
+! CHECK-NEXT: {{.*}}/debug-parsing-log.f90:31:1: in the context: IMPLICIT statement
+! CHECK-NEXT:   END PROGRAM
+! CHECK-NEXT:   ^
+! CHECK-NEXT: {{.*}}/debug-parsing-log.f90:31:1: in the context: implicit part
+! CHECK-NEXT:   END PROGRAM
+! CHECK-NEXT:   ^
+! CHECK-NEXT: {{.*}}/debug-parsing-log.f90:31:1: in the context: specification part
+! CHECK-NEXT:   END PROGRAM
+! CHECK-NEXT:   ^
+! CHECK-NEXT: {{.*}}/debug-parsing-log.f90:31:1: in the context: main program
+! CHECK-NEXT:   END PROGRAM
+! CHECK-NEXT:   ^
+
+!-----------------
+! TEST INPUT
+!-----------------
+END PROGRAM

flang/test/Driver/driver-help.f90

 ! HELP-FC1-NEXT:OPTIONS:
 ! HELP-FC1-NEXT: -D <macro>=<value>     Define <macro> to <value> (or 1 if <value> omitted)
 ! HELP-FC1-NEXT: -emit-obj Emit native object files
 ! HELP-FC1-NEXT: -E                     Only run the preprocessor
 ! HELP-FC1-NEXT: -falternative-parameter-statement
 ! HELP-FC1-NEXT: Enable the old style PARAMETER statement
 ! HELP-FC1-NEXT: -fbackslash            Specify that backslash in string introduces an escape character
 ! HELP-FC1-NEXT: -fdebug-dump-parse-tree Dump the parse tree
+! HELP-FC1-NEXT: -fdebug-dump-parsing-log
+! HELP-FC1-NEXT:                   Run instrumented parse and dump the parsing log
 ! HELP-FC1-NEXT: -fdebug-dump-provenance Dump provenance
 ! HELP-FC1-NEXT: -fdebug-dump-symbols    Dump symbols after the semantic analysis
 ! HELP-FC1-NEXT: -fdebug-measure-parse-tree
 ! HELP-FC1-NEXT:                         Measure the parse tree
 ! HELP-FC1-NEXT: -fdebug-pre-fir-tree    Dump the pre-FIR tree
 ! HELP-FC1-NEXT: -fdebug-unparse-with-symbols
 ! HELP-FC1-NEXT:                        Unparse and stop.
 ! HELP-FC1-NEXT: -fdebug-unparse        Unparse and stop.

flang/tools/f18/f18.cpp

if (arg.empty()) {
       driver.dumpPreFirTree = true;
     } else if (arg == "-fdebug-dump-symbols") {
       driver.dumpSymbols = true;
       driver.syntaxOnly = true;
     } else if (arg == "-fdebug-module-writer") {
       driver.debugModuleWriter = true;
     } else if (arg == "-fdebug-measure-parse-tree") {
       driver.measureTree = true;
-    } else if (arg == "-fdebug-instrumented-parse") {
+    } else if (arg == "-fdebug-instrumented-parse" ||
+        arg == "-fdebug-dump-parsing-log") {
       options.instrumentedParse = true;
     } else if (arg == "-fdebug-no-semantics") {
       driver.debugNoSemantics = true;
     } else if (arg == "-funparse" || arg == "-fdebug-unparse") {
       driver.dumpUnparse = true;
     } else if (arg == "-funparse-with-symbols" ||
         arg == "-fdebug-unparse-with-symbols") {
       driver.dumpUnparseWithSymbols = true;