[flang][driver] Refactor methods for parsing options (nfc)

[awarzynski]

Summary

This is just a small update that makes sure that errors arising from
parsing command-line options are captured more visibly. Also, all
parsing methods will now consistently return either a bool ("may fail")
or void ("never fails").

An instance of InputKind coming from -x is added to
FrontendOptions rather then being returned from ParseFrontendArgs.
It's currently not used, but we will require it shortly. In particular,
once code-generation is available we will use it to differentiate
between LLVM IR and Fortran input. FrontendOptions is a very suitable
place to keep it.

This changes don't affect the error reporting in the driver. In this
respect these are non-functional-changes. However, it will simplify
things in the forthcoming patches in which we may need a better error
tracking/recovery mechanism.

[SouraVX]

LG - Thanks! with one question inline :)

flang/include/flang/Frontend/FrontendOptions.h

225

Out of curiosity, what does clang does for this ?

clang -x c++ main.c

Is sharing possible ?

[SouraVX]

flang/include/flang/Frontend/FrontendOptions.h

225

I meant what clangDriver does, for handling this ? Code sharing possible ?

[awarzynski]

Thank you for reviewing @SouraVX !

flang/include/flang/Frontend/FrontendOptions.h

225

I meant what clangDriver does, for handling this ? Code sharing possible ?

Not without some non-trivial refactor. This code is for the frontend driver. clangDriver is only used for the compiler driver. It's the same in Clang.

I agree that it would be nice to share some code from the frontend driver with Clang. And in theory it shouldn't be too difficult - one would simply create a few generic/abstract classes that Flang and Clang would specialise. But the devil is going to be in the details. Also, we need a volunteer first :)

flang/include/flang/Frontend/FrontendOptions.h

public:
 
   // The form to process files in, if specified.
   FortranForm fortranForm_ = FortranForm::Unknown;
 
   // The column after which characters are ignored in fixed form lines in the
   // source file.
   int fixedFormColumns_ = 72;
 
+  /// The input kind, either specified via -x argument or deduced from the input
+  /// file name.
+  InputKind dashX_;
+
   // Language features
   common::LanguageFeatureControl features_;
 
   // Source file encoding
   Fortran::parser::Encoding encoding_{Fortran::parser::Encoding::UTF_8};
 
 public:
   FrontendOptions()

flang/lib/Frontend/CompilerInvocation.cpp

static void setUpFrontendBasedOnAction(FrontendOptions &opts) {
 
   if (opts.programAction_ == DebugDumpParsingLog)
     opts.instrumentedParse_ = true;
 
   if (opts.programAction_ == DebugDumpProvenance)
     opts.needProvenanceRangeToCharBlockMappings_ = true;
 }
 
-static InputKind ParseFrontendArgs(FrontendOptions &opts,
-    llvm::opt::ArgList &args, clang::DiagnosticsEngine &diags) {
+static void 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 =
           args.getLastArg(clang::driver::options::OPT_Action_Group)) {
     switch (a->getOption().getID()) {
if (argValue == "utf-8") {
       opts.encoding_ = Fortran::parser::Encoding::LATIN_1;
     } else {
       diags.Report(clang::diag::err_drv_invalid_value)
           << arg->getAsString(args) << argValue;
     }
   }
 
   setUpFrontendBasedOnAction(opts);
-
-  return dashX;
+  opts.dashX_ = dashX;
 }
 
 // Generate the path to look for intrinsic modules
 static std::string getIntrinsicDir() {
   // TODO: Find a system independent API
   llvm::SmallString<128> driverPath;
   driverPath.assign(llvm::sys::fs::getMainExecutable(nullptr, nullptr));
   llvm::sys::path::remove_filename(driverPath);
if (const auto *currentArg = args.getLastArg(
           clang::driver::options::OPT_cpp, clang::driver::options::OPT_nocpp))
     opts.macrosFlag_ =
         (currentArg->getOption().matches(clang::driver::options::OPT_cpp))
         ? PPMacrosFlag::Include
         : PPMacrosFlag::Exclude;
 }
 
 /// Parses all semantic related arguments and populates the variables
-/// options accordingly.
-static void parseSemaArgs(CompilerInvocation &res, llvm::opt::ArgList &args,
+/// options accordingly. Returns false if new errors are generated.
+static bool parseSemaArgs(CompilerInvocation &res, llvm::opt::ArgList &args,
     clang::DiagnosticsEngine &diags) {
+  unsigned numErrorsBefore = diags.getNumErrors();
 
   // -J/module-dir option
   auto moduleDirList =
       args.getAllArgValues(clang::driver::options::OPT_module_dir);
   // User can only specify -J/-module-dir once
   // https://gcc.gnu.org/onlinedocs/gfortran/Directory-Options.html
   if (moduleDirList.size() > 1) {
     const unsigned diagID =
         diags.getCustomDiagID(clang::DiagnosticsEngine::Error,
             "Only one '-module-dir/-J' option allowed");
     diags.Report(diagID);
   }
   if (moduleDirList.size() == 1)
     res.SetModuleDir(moduleDirList[0]);
 
   // -fdebug-module-writer option
   if (args.hasArg(clang::driver::options::OPT_fdebug_module_writer)) {
     res.SetDebugModuleDir(true);
   }
+
+  return diags.getNumErrors() == numErrorsBefore;
 }
 
 /// Parses all diagnostics related arguments and populates the variables
-/// options accordingly.
-static void parseDiagArgs(CompilerInvocation &res, llvm::opt::ArgList &args,
+/// options accordingly. Returns false if new errors are generated.
+static bool parseDiagArgs(CompilerInvocation &res, llvm::opt::ArgList &args,
     clang::DiagnosticsEngine &diags) {
+  unsigned numErrorsBefore = diags.getNumErrors();
+
   // -Werror option
   // TODO: Currently throws a Diagnostic for anything other than -W<error>,
   // this has to change when other -W<opt>'s are supported.
   if (args.hasArg(clang::driver::options::OPT_W_Joined)) {
     if (args.getLastArgValue(clang::driver::options::OPT_W_Joined)
             .equals("error")) {
       res.SetWarnAsErr(true);
     } else {
       const unsigned diagID =
           diags.getCustomDiagID(clang::DiagnosticsEngine::Error,
               "Only `-Werror` is supported currently.");
       diags.Report(diagID);
     }
   }
+
+  return diags.getNumErrors() == numErrorsBefore;
 }
 
 /// Parses all Dialect related arguments and populates the variables
-/// options accordingly.
-static void parseDialectArgs(CompilerInvocation &res, llvm::opt::ArgList &args,
+/// options accordingly. Returns false if new errors are generated.
+static bool parseDialectArgs(CompilerInvocation &res, llvm::opt::ArgList &args,
     clang::DiagnosticsEngine &diags) {
+  unsigned numErrorsBefore = diags.getNumErrors();
 
   // -fdefault* family
   if (args.hasArg(clang::driver::options::OPT_fdefault_real_8)) {
     res.defaultKinds().set_defaultRealKind(8);
     res.defaultKinds().set_doublePrecisionKind(16);
   }
   if (args.hasArg(clang::driver::options::OPT_fdefault_integer_8)) {
     res.defaultKinds().set_defaultIntegerKind(8);
if (standard.equals("f2018")) {
       res.set_EnableConformanceChecks();
     } else {
       const unsigned diagID =
           diags.getCustomDiagID(clang::DiagnosticsEngine::Error,
               "Only -std=f2018 is allowed currently.");
       diags.Report(diagID);
     }
   }
-  return;
+  return diags.getNumErrors() == numErrorsBefore;
 }
 
 bool CompilerInvocation::CreateFromArgs(CompilerInvocation &res,
     llvm::ArrayRef<const char *> commandLineArgs,
     clang::DiagnosticsEngine &diags) {
 
   bool success = true;
 
for (const auto *a : args.filtered(clang::driver::options::OPT_UNKNOWN)) {
     else
       diags.Report(clang::diag::err_drv_unknown_argument_with_suggestion)
           << argString << nearest;
     success = false;
   }
 
   // Parse the frontend args
   ParseFrontendArgs(res.frontendOpts(), args, diags);
-  // Parse the preprocessor args
   parsePreprocessorArgs(res.preprocessorOpts(), args);
-  // Parse semantic args
-  parseSemaArgs(res, args, diags);
-  // Parse dialect arguments
-  parseDialectArgs(res, args, diags);
-  // Parse diagnostic arguments
-  parseDiagArgs(res, args, diags);
+  success &= parseSemaArgs(res, args, diags);
+  success &= parseDialectArgs(res, args, diags);
+  success &= parseDiagArgs(res, args, diags);
 
   return success;
 }
 
 void CompilerInvocation::collectMacroDefinitions() {
   auto &ppOpts = this->preprocessorOpts();
 
   for (unsigned i = 0, n = ppOpts.macros.size(); i != n; ++i) {