[clang][flang] Improve the consistency of the code-base

[achieveartificialintelligence]

Summary

In clang:

• Replace argc_ with Argc
• Replace argv_ with Argv
• Replace argv with Args

In flang:

• Replace argc_ with argc
• Replace argv_ with argv
• Replace argv with args

[jhenderson]

In general, I don't work on the clang side. As such, I don't know the norms surrounding this sort of change in this area, and don't feel comfortable reviewing. You should look at getting reviewers who do work in that part of the repository.

[awarzynski]

Thank you for submitting this @achieveartificialintelligence !

The aim of these changes is to improve the consistency of the code-base with respect to the coding standards documented separately for Clang and Flang. I think that this is a very positive suggestion, but would like to request a few refinements before this is ready:

• Please note that in LLVM/Clang, variables names start with upper case (so it should be int main(int Argc, const char **Argv) instead of main(int argc, const char **argv) ). link
• In Flang the rules are slightly different, and the variable names start with a lower case (so it should be int main(int Argc, const char **Argv) instead of main(int argc, const char **argv) ). link
• Your commit message refers to _a driver_, but in fact you are updating 2 specific drivers: Clang compiler driver and Flang compiler driver - could you clarify?
• Your commit message refers to argc_ specifically, but you are updating argc_, argv_ and Argv/argv too.

Personally I'd prefer ArgC/argC and ArgV/argV for input parameters and ArgValues/argValues for the local variable. This way it would be more self-explanatory. But that's a matter of style. As long as this is consistent with the coding standards, I think that this should be accepted.

[achieveartificialintelligence]

In D97138#2578563, @awarzynski wrote:

Thank you for submitting this @achieveartificialintelligence !

The aim of these changes is to improve the consistency of the code-base with respect to the coding standards documented separately for Clang and Flang. I think that this is a very positive suggestion, but would like to request a few refinements before this is ready:

• Please note that in LLVM/Clang, variables names start with upper case (so it should be int main(int Argc, const char **Argv) instead of main(int argc, const char **argv) ). link
• In Flang the rules are slightly different, and the variable names start with a lower case (so it should be int main(int Argc, const char **Argv) instead of main(int argc, const char **argv) ). link
• Your commit message refers to _a driver_, but in fact you are updating 2 specific drivers: Clang compiler driver and Flang compiler driver - could you clarify?
• Your commit message refers to argc_ specifically, but you are updating argc_, argv_ and Argv/argv too.

Personally I'd prefer ArgC/argC and ArgV/argV for input parameters and ArgValues/argValues for the local variable. This way it would be more self-explanatory. But that's a matter of style. As long as this is consistent with the coding standards, I think that this should be accepted.

Thank you very much for your comment.

[aganea]

clang/tools/driver/driver.cpp

349

What about just Args? "Values" sounds a bit too broad.

[achieveartificialintelligence]

Thanks @aganea. And I have a question that, should I use argc & argv or argC & argV in the flang part now ?

[aganea]

In D97138#2586988, @achieveartificialintelligence wrote:

Thanks @aganea. And I have a question that, should I use argc & argv or argC & argV in the flang part now ?

Looks good as it is now, thanks!

[awarzynski]

Thank you, this is a much appreciated improvement!

clang/tools/driver/driver.cpp

if (Tool == "-cc1gen-reproducer")
     return cc1gen_reproducer_main(makeArrayRef(ArgV).slice(2), ArgV[0],
                                   GetExecutablePathVP);
   // Reject unknown tools.
   llvm::errs() << "error: unknown integrated tool '" << Tool << "'. "
                << "Valid tools include '-cc1' and '-cc1as'.\n";
   return 1;
 }
 
-int main(int argc_, const char **argv_) {
+int main(int Argc, const char **Argv) {
   noteBottomOfStack();
-  llvm::InitLLVM X(argc_, argv_);
+  llvm::InitLLVM X(Argc, Argv);
   llvm::setBugReportMsg("PLEASE submit a bug report to " BUG_REPORT_URL
                         " and include the crash backtrace, preprocessed "
                         "source, and associated run script.\n");
-  SmallVector<const char *, 256> argv(argv_, argv_ + argc_);
+  SmallVector<const char *, 256> Args(Argv, Argv + Argc);
 
   if (llvm::sys::Process::FixupStandardFileDescriptors())
     return 1;
 
   llvm::InitializeAllTargets();
-  auto TargetAndMode = ToolChain::getTargetAndModeFromProgramName(argv[0]);
+  auto TargetAndMode = ToolChain::getTargetAndModeFromProgramName(Args[0]);
 
   llvm::BumpPtrAllocator A;
   llvm::StringSaver Saver(A);
 
   // Parse response files using the GNU syntax, unless we're in CL mode. There
-  // are two ways to put clang in CL compatibility mode: argv[0] is either
+  // are two ways to put clang in CL compatibility mode: Args[0] is either
   // clang-cl or cl, or --driver-mode=cl is on the command line. The normal
   // command line parsing can't happen until after response file parsing, so we
   // have to manually search for a --driver-mode=cl argument the hard way.
   // Finally, our -cc1 tools don't care which tokenization mode we use because
   // response files written by clang will tokenize the same way in either mode.
   bool ClangCLMode = false;
   if (StringRef(TargetAndMode.DriverMode).equals("--driver-mode=cl") ||
-      llvm::find_if(argv, [](const char *F) {
+      llvm::find_if(Args, [](const char *F) {
         return F && strcmp(F, "--driver-mode=cl") == 0;
-      }) != argv.end()) {
+      }) != Args.end()) {
     ClangCLMode = true;
   }
   enum { Default, POSIX, Windows } RSPQuoting = Default;
-  for (const char *F : argv) {
+  for (const char *F : Args) {
     if (strcmp(F, "--rsp-quoting=posix") == 0)
       RSPQuoting = POSIX;
     else if (strcmp(F, "--rsp-quoting=windows") == 0)
       RSPQuoting = Windows;
   }
 
-  // Determines whether we want nullptr markers in argv to indicate response
+  // Determines whether we want nullptr markers in Args to indicate response
   // files end-of-lines. We only use this for the /LINK driver argument with
   // clang-cl.exe on Windows.
   bool MarkEOLs = ClangCLMode;
 
   llvm::cl::TokenizerCallback Tokenizer;
   if (RSPQuoting == Windows || (RSPQuoting == Default && ClangCLMode))
     Tokenizer = &llvm::cl::TokenizeWindowsCommandLine;
   else
     Tokenizer = &llvm::cl::TokenizeGNUCommandLine;
 
-  if (MarkEOLs && argv.size() > 1 && StringRef(argv[1]).startswith("-cc1"))
+  if (MarkEOLs && Args.size() > 1 && StringRef(Args[1]).startswith("-cc1"))
     MarkEOLs = false;
-  llvm::cl::ExpandResponseFiles(Saver, Tokenizer, argv, MarkEOLs);
+  llvm::cl::ExpandResponseFiles(Saver, Tokenizer, Args, MarkEOLs);
 
   // Handle -cc1 integrated tools, even if -cc1 was expanded from a response
   // file.
-  auto FirstArg = std::find_if(argv.begin() + 1, argv.end(),
+  auto FirstArg = std::find_if(Args.begin() + 1, Args.end(),
                                [](const char *A) { return A != nullptr; });
-  if (FirstArg != argv.end() && StringRef(*FirstArg).startswith("-cc1")) {
+  if (FirstArg != Args.end() && StringRef(*FirstArg).startswith("-cc1")) {
     // If -cc1 came from a response file, remove the EOL sentinels.
     if (MarkEOLs) {
-      auto newEnd = std::remove(argv.begin(), argv.end(), nullptr);
-      argv.resize(newEnd - argv.begin());
+      auto newEnd = std::remove(Args.begin(), Args.end(), nullptr);
+      Args.resize(newEnd - Args.begin());
     }
-    return ExecuteCC1Tool(argv);
+    return ExecuteCC1Tool(Args);
   }
 
   // Handle options that need handling before the real command line parsing in
   // Driver::BuildCompilation()
   bool CanonicalPrefixes = true;
-  for (int i = 1, size = argv.size(); i < size; ++i) {
+  for (int i = 1, size = Args.size(); i < size; ++i) {
     // Skip end-of-line response file markers
-    if (argv[i] == nullptr)
+    if (Args[i] == nullptr)
       continue;
-    if (StringRef(argv[i]) == "-no-canonical-prefixes") {
+    if (StringRef(Args[i]) == "-no-canonical-prefixes") {
       CanonicalPrefixes = false;
       break;
     }
   }
 
   // Handle CL and _CL_ which permits additional command line options to be
   // prepended or appended.
   if (ClangCLMode) {
     // Arguments in "CL" are prepended.
     llvm::Optional<std::string> OptCL = llvm::sys::Process::GetEnv("CL");
     if (OptCL.hasValue()) {
       SmallVector<const char *, 8> PrependedOpts;
       getCLEnvVarOptions(OptCL.getValue(), Saver, PrependedOpts);
 
       // Insert right after the program name to prepend to the argument list.
-      argv.insert(argv.begin() + 1, PrependedOpts.begin(), PrependedOpts.end());
+      Args.insert(Args.begin() + 1, PrependedOpts.begin(), PrependedOpts.end());
     }
     // Arguments in "_CL_" are appended.
     llvm::Optional<std::string> Opt_CL_ = llvm::sys::Process::GetEnv("_CL_");
     if (Opt_CL_.hasValue()) {
       SmallVector<const char *, 8> AppendedOpts;
       getCLEnvVarOptions(Opt_CL_.getValue(), Saver, AppendedOpts);
 
       // Insert at the end of the argument list to append.
-      argv.append(AppendedOpts.begin(), AppendedOpts.end());
+      Args.append(AppendedOpts.begin(), AppendedOpts.end());
     }
   }
 
   std::set<std::string> SavedStrings;
   // Handle CCC_OVERRIDE_OPTIONS, used for editing a command line behind the
   // scenes.
   if (const char *OverrideStr = ::getenv("CCC_OVERRIDE_OPTIONS")) {
     // FIXME: Driver shouldn't take extra initial argument.
-    ApplyQAOverride(argv, OverrideStr, SavedStrings);
+    ApplyQAOverride(Args, OverrideStr, SavedStrings);
   }
 
-  std::string Path = GetExecutablePath(argv[0], CanonicalPrefixes);
+  std::string Path = GetExecutablePath(Args[0], CanonicalPrefixes);
 
   // Whether the cc1 tool should be called inside the current process, or if we
   // should spawn a new clang subprocess (old behavior).
   // Not having an additional process saves some execution time of Windows,
   // and makes debugging and profiling easier.
   bool UseNewCC1Process;
 
   IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts =
-      CreateAndPopulateDiagOpts(argv, UseNewCC1Process);
+      CreateAndPopulateDiagOpts(Args, UseNewCC1Process);
 
   TextDiagnosticPrinter *DiagClient
     = new TextDiagnosticPrinter(llvm::errs(), &*DiagOpts);
   FixupDiagPrefixExeName(DiagClient, Path);
 
   IntrusiveRefCntPtr<DiagnosticIDs> DiagID(new DiagnosticIDs());
 
   DiagnosticsEngine Diags(DiagID, &*DiagOpts, DiagClient);
 
   if (!DiagOpts->DiagnosticSerializationFile.empty()) {
     auto SerializedConsumer =
         clang::serialized_diags::create(DiagOpts->DiagnosticSerializationFile,
                                         &*DiagOpts, /*MergeChildRecords=*/true);
     Diags.setClient(new ChainedDiagnosticConsumer(
         Diags.takeClient(), std::move(SerializedConsumer)));
   }
 
   ProcessWarningOptions(Diags, *DiagOpts, /*ReportDiags=*/false);
 
   Driver TheDriver(Path, llvm::sys::getDefaultTargetTriple(), Diags);
-  SetInstallDir(argv, TheDriver, CanonicalPrefixes);
+  SetInstallDir(Args, TheDriver, CanonicalPrefixes);
   TheDriver.setTargetAndMode(TargetAndMode);
 
-  insertTargetAndModeArgs(TargetAndMode, argv, SavedStrings);
+  insertTargetAndModeArgs(TargetAndMode, Args, SavedStrings);
 
   SetBackdoorDriverOutputsFromEnvVars(TheDriver);
 
   if (!UseNewCC1Process) {
     TheDriver.CC1Main = &ExecuteCC1Tool;
     // Ensure the CC1Command actually catches cc1 crashes
     llvm::CrashRecoveryContext::Enable();
   }
 
-  std::unique_ptr<Compilation> C(TheDriver.BuildCompilation(argv));
+  std::unique_ptr<Compilation> C(TheDriver.BuildCompilation(Args));
   int Res = 1;
   bool IsCrash = false;
   if (C && !C->containsError()) {
     SmallVector<std::pair<int, const Command *>, 4> FailingCommands;
     Res = TheDriver.ExecuteCompilation(*C, FailingCommands);
 
     // Force a crash to test the diagnostics.
     if (TheDriver.GenReproducer) {

flang/tools/flang-driver/driver.cpp

static int ExecuteFC1Tool(llvm::SmallVectorImpl<const char *> &argV) {
 
   // Reject unknown tools.
   // ATM it only supports fc1. Any fc1[*] is rejected.
   llvm::errs() << "error: unknown integrated tool '" << tool << "'. "
                << "Valid tools include '-fc1'.\n";
   return 1;
 }
 
-int main(int argc_, const char **argv_) {
+int main(int argc, const char **argv) {
 
   // Initialize variables to call the driver
-  llvm::InitLLVM x(argc_, argv_);
-  llvm::SmallVector<const char *, 256> argv(argv_, argv_ + argc_);
+  llvm::InitLLVM x(argc, argv);
+  llvm::SmallVector<const char *, 256> args(argv, argv + argc);
 
   clang::driver::ParsedClangName targetandMode("flang", "--driver-mode=flang");
-  std::string driverPath = GetExecutablePath(argv[0]);
+  std::string driverPath = GetExecutablePath(args[0]);
 
   // Check if flang-new is in the frontend mode
   auto firstArg = std::find_if(
-      argv.begin() + 1, argv.end(), [](const char *a) { return a != nullptr; });
-  if (firstArg != argv.end()) {
-    if (llvm::StringRef(argv[1]).startswith("-cc1")) {
-      llvm::errs() << "error: unknown integrated tool '" << argv[1] << "'. "
+      args.begin() + 1, args.end(), [](const char *a) { return a != nullptr; });
+  if (firstArg != args.end()) {
+    if (llvm::StringRef(args[1]).startswith("-cc1")) {
+      llvm::errs() << "error: unknown integrated tool '" << args[1] << "'. "
                    << "Valid tools include '-fc1'.\n";
       return 1;
     }
     // Call flang-new frontend
-    if (llvm::StringRef(argv[1]).startswith("-fc1")) {
-      return ExecuteFC1Tool(argv);
+    if (llvm::StringRef(args[1]).startswith("-fc1")) {
+      return ExecuteFC1Tool(args);
     }
   }
 
   // Not in the frontend mode - continue in the compiler driver mode.
 
   // Create DiagnosticsEngine for the compiler driver
   llvm::IntrusiveRefCntPtr<clang::DiagnosticOptions> diagOpts =
-      CreateAndPopulateDiagOpts(argv);
+      CreateAndPopulateDiagOpts(args);
   llvm::IntrusiveRefCntPtr<clang::DiagnosticIDs> diagID(
       new clang::DiagnosticIDs());
   Fortran::frontend::TextDiagnosticPrinter *diagClient =
       new Fortran::frontend::TextDiagnosticPrinter(llvm::errs(), &*diagOpts);
 
   diagClient->set_prefix(
-      std::string(llvm::sys::path::stem(GetExecutablePath(argv[0]))));
+      std::string(llvm::sys::path::stem(GetExecutablePath(args[0]))));
 
   clang::DiagnosticsEngine diags(diagID, &*diagOpts, diagClient);
 
   // Prepare the driver
   clang::driver::Driver theDriver(driverPath,
       llvm::sys::getDefaultTargetTriple(), diags, "flang LLVM compiler");
   theDriver.setTargetAndMode(targetandMode);
   std::unique_ptr<clang::driver::Compilation> c(
-      theDriver.BuildCompilation(argv));
+      theDriver.BuildCompilation(args));
   llvm::SmallVector<std::pair<int, const clang::driver::Command *>, 4>
       failingCommands;
 
   // Run the driver
   int res = 1;
   bool isCrash = false;
   res = theDriver.ExecuteCompilation(*c, failingCommands);