[flang][driver] NFC: Make code more in line with LLVM style

[awarzynski]

Summary

This patch basically implements [1] in ExecuteCompilerInvocation.cpp. It
also:

• replaces CreateFrontendBaseAction with CreateFrontendAction (only one method is needed ATM, this change removes the extra indirection)
• removes InvalidAction from the ActionKind enum (I don't think it adds much and keeping it would mean adding a new void case in CreateFrontendAction)

No new functionality is added, hence no tests.

[1] https://llvm.org/docs/CodingStandards.html#don-t-use-default-labels-in-fully-covered-switches-over-enumerations

[unterumarmung]

LGTM

[ekieri]

Thanks for taking the time to work on quality and compliance! LGTM, but please consider my nits.

The removal of InvalidAction made me worry about the default. If I understood correctly, the default programAction is set (safely) here. But would it not feel more secure to set it directly in the definition instead? Anyway, makes sense to me to remove InvalidAction. A sensible default is more helpful than a failing default.

flang/include/flang/FrontendTool/Utils.h

19–20

Then this forward declaration should go as well.

And please remove the #include from FrontendAction.cpp. An unrelated redundancy, but somewhat related :)

[awarzynski]

Address Emil's nits.

[awarzynski]

Thanks for taking a look, @ekieri !

In D124245#3469632, @ekieri wrote:

The removal of InvalidAction made me worry about the default. If I understood correctly, the default programAction is set (safely) here. But would it not feel more secure to set it directly in the definition instead? Anyway, makes sense to me to remove InvalidAction. A sensible default is more helpful than a failing default.

These are good points, thanks! I would still like to set programAction in ParseFrontendArg (on top of having a default in FrontendOptions.h) - that's where other parts of CompilerInvocation are set. Also, this way the defaults in FrontendOptions.h don't matter as long as they are sane. WDYT?

[ekieri]

Thanks for addressing my comments!

In D124245#3475038, @awarzynski wrote:

These are good points, thanks! I would still like to set programAction in ParseFrontendArg (on top of having a default in FrontendOptions.h) - that's where other parts of CompilerInvocation are set. Also, this way the defaults in FrontendOptions.h don't matter as long as they are sane. WDYT?

Sure, those are good arguments, I have no problem with this solution.

flang/include/flang/Frontend/FrontendOptions.h

 #include "llvm/Support/MemoryBuffer.h"
 
 #include <cstdint>
 #include <string>
 
 namespace Fortran::frontend {
 
 enum ActionKind {
-  InvalidAction = 0,
-
   /// -test-io mode
   InputOutputTest,
 
   /// -E mode
   PrintPreprocessedInput,
 
   /// -fsyntax-only
   ParseSyntaxOnly,
struct FrontendOptions {
 
   /// 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;
+  frontend::ActionKind programAction = ParseSyntaxOnly;
 
   // 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;
 

flang/include/flang/FrontendTool/Utils.h

 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_FLANG_FRONTENDTOOL_UTILS_H
 #define LLVM_FLANG_FRONTENDTOOL_UTILS_H
 
 namespace Fortran::frontend {
 
 class CompilerInstance;
-class FrontendAction;
-
-/// Construct the FrontendAction of a compiler invocation based on the
-/// options specified for the compiler invocation.
-///
-/// \return - The created FrontendAction object
-std::unique_ptr<FrontendAction> CreateFrontendAction(CompilerInstance &ci);
 
 /// ExecuteCompilerInvocation - Execute the given actions described by the
 /// compiler invocation object in the given compiler instance.
 ///
 /// \return - True on success.
 bool ExecuteCompilerInvocation(CompilerInstance *flang);
 
 } // end namespace Fortran::frontend
 
 #endif // LLVM_FLANG_FRONTENDTOOL_UTILS_H

flang/lib/Frontend/CompilerInvocation.cpp

 /// \param [in] opts The target options instance to update
 /// \param [in] args The list of input arguments (from the compiler invocation)
 static void ParseTargetArgs(TargetOptions &opts, llvm::opt::ArgList &args) {
   opts.triple = args.getLastArgValue(clang::driver::options::OPT_triple);
 }
 
 // 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;
 
   if (opts.programAction == DebugDumpProvenance ||
       opts.programAction == Fortran::frontend::GetDefinition)
     opts.needProvenanceRangeToCharBlockMappings = true;
 }
 

flang/lib/Frontend/FrontendAction.cpp

 //===--- FrontendAction.cpp -----------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "flang/Frontend/FrontendAction.h"
 #include "flang/Frontend/CompilerInstance.h"
 #include "flang/Frontend/FrontendActions.h"
 #include "flang/Frontend/FrontendOptions.h"
 #include "flang/Frontend/FrontendPluginRegistry.h"
-#include "flang/FrontendTool/Utils.h"
 #include "clang/Basic/DiagnosticFrontend.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/VirtualFileSystem.h"
 
 using namespace Fortran::frontend;
 
 LLVM_INSTANTIATE_REGISTRY(FrontendPluginRegistry)
 

flang/lib/FrontendTool/ExecuteCompilerInvocation.cpp

 #include "llvm/Option/Option.h"
 #include "llvm/Support/BuryPointer.h"
 #include "llvm/Support/CommandLine.h"
 #include "mlir/IR/MLIRContext.h"
 #include "mlir/Pass/PassManager.h"
 
 namespace Fortran::frontend {
 
-static std::unique_ptr<FrontendAction> CreateFrontendBaseAction(
+static std::unique_ptr<FrontendAction> CreateFrontendAction(
     CompilerInstance &ci) {
 
-  ActionKind ak = ci.frontendOpts().programAction;
-  switch (ak) {
+  switch (ci.frontendOpts().programAction) {
   case InputOutputTest:
     return std::make_unique<InputOutputTestAction>();
   case PrintPreprocessedInput:
     return std::make_unique<PrintPreprocessedAction>();
   case ParseSyntaxOnly:
     return std::make_unique<ParseSyntaxOnlyAction>();
   case EmitMLIR:
     return std::make_unique<EmitMLIRAction>();
for (const FrontendPluginRegistry::entry &plugin :
         return std::move(p);
       }
     }
     unsigned diagID = ci.diagnostics().getCustomDiagID(
         clang::DiagnosticsEngine::Error, "unable to find plugin '%0'");
     ci.diagnostics().Report(diagID) << ci.frontendOpts().ActionName;
     return nullptr;
   }
-  default:
-    break;
-    // TODO:
-    // case ParserSyntaxOnly:
-    // case EmitLLVM:
-    // case EmitLLVMOnly:
-    // case EmitCodeGenOnly:
-    // (...)
-  }
-  return 0;
   }
 
-std::unique_ptr<FrontendAction> CreateFrontendAction(CompilerInstance &ci) {
-  // Create the underlying action.
-  std::unique_ptr<FrontendAction> act = CreateFrontendBaseAction(ci);
-  if (!act)
-    return nullptr;
-
-  return act;
+  llvm_unreachable("Invalid program action!");
 }
 
 bool ExecuteCompilerInvocation(CompilerInstance *flang) {
   // Honor -help.
   if (flang->frontendOpts().showHelp) {
     clang::driver::getDriverOptTable().printHelp(llvm::outs(),
         "flang-new -fc1 [options] file...", "LLVM 'Flang' Compiler",
         /*Include=*/clang::driver::options::FC1Option,