Change some series of AddAllArgs calls to use the most general AddAlllArgs.

[dougk]

Summary

In a bunch of places where we had several calls to AddAllArgs, the new overload accepting an ArrayRef can be used.

But there is actually an observable difference: the old way re-ordered the rendered arguments across groups; but within a single group, they appear in-order as originally expressed.
The new behavior preserves original order throughout, and I assume that this is generally to be preferred.
It would be strange indeed if the driver was required to alter the order as written (at least not without a comment saying "We need to reorder these...").
Which is to say, I think that being faithful to what the user supplied should not be wrong.

Patch depends on http://reviews.llvm.org/D11597

[dougk]

lib/Driver/Tools.cpp

6516 ↗

(On Diff #30914)

oops. an 'AddLastArg' got mixed in here.
But, it seems that most other places do add *all* of the 'e' args.
Which is right?

[chandlerc]

Very much prefer preserving the original order.

We were already working around the reorderings by putting the D and U groups into a single call. =/ This approach seems *much* cleaner.

cfe/trunk/lib/Driver/Tools.cpp

if (A->getOption().matches(options::OPT_include)) {
       }
     }
 
     // Not translated, render as usual.
     A->claim();
     A->render(Args, CmdArgs);
   }
 
-  Args.AddAllArgs(CmdArgs, options::OPT_D, options::OPT_U);
-  Args.AddAllArgs(CmdArgs, options::OPT_I_Group, options::OPT_F,
-                  options::OPT_index_header_map);
+  Args.AddAllArgs(CmdArgs,
+                  {options::OPT_D, options::OPT_U, options::OPT_I_Group,
+                   options::OPT_F, options::OPT_index_header_map});
 
   // Add -Wp, and -Xassembler if using the preprocessor.
 
   // FIXME: There is a very unfortunate problem here, some troubled
   // souls abuse -Wp, to pass preprocessor options in gcc syntax. To
   // really support that we would have to parse and then translate
   // those options. :(
   Args.AddAllArgValues(CmdArgs, options::OPT_Wp_COMMA,
constructHexagonLinkArgs(Compilation &C, const JobAction &JA,
   //----------------------------------------------------------------------------
   const ToolChain::path_list &LibPaths = ToolChain.getFilePaths();
   for (const auto &LibPath : LibPaths)
     CmdArgs.push_back(Args.MakeArgString(StringRef("-L") + LibPath));
 
   //----------------------------------------------------------------------------
   //
   //----------------------------------------------------------------------------
-  Args.AddAllArgs(CmdArgs, options::OPT_T_Group);
-  Args.AddAllArgs(CmdArgs, options::OPT_e);
-  Args.AddAllArgs(CmdArgs, options::OPT_s);
-  Args.AddAllArgs(CmdArgs, options::OPT_t);
-  Args.AddAllArgs(CmdArgs, options::OPT_u_Group);
+  Args.AddAllArgs(CmdArgs,
+                  {options::OPT_T_Group, options::OPT_e, options::OPT_s,
+                   options::OPT_t, options::OPT_u_Group});
 
   AddLinkerInputs(ToolChain, Inputs, Args, CmdArgs);
 
   //----------------------------------------------------------------------------
   // Libraries
   //----------------------------------------------------------------------------
   if (incStdLib && incDefLibs) {
     if (D.CCCIsCXX()) {
if (!Args.hasArg(options::OPT_nostdlib) &&
     CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("crt0.o")));
     CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("crtbegin.o")));
   }
 
   Args.AddAllArgs(CmdArgs, options::OPT_L);
   const ToolChain::path_list &Paths = ToolChain.getFilePaths();
   for (const auto &Path : Paths)
     CmdArgs.push_back(Args.MakeArgString(StringRef("-L") + Path));
-  Args.AddAllArgs(CmdArgs, options::OPT_T_Group);
-  Args.AddAllArgs(CmdArgs, options::OPT_e);
-  Args.AddAllArgs(CmdArgs, options::OPT_s);
-  Args.AddAllArgs(CmdArgs, options::OPT_t);
-  Args.AddAllArgs(CmdArgs, options::OPT_Z_Flag);
-  Args.AddAllArgs(CmdArgs, options::OPT_r);
+  Args.AddAllArgs(CmdArgs,
+                  {options::OPT_T_Group, options::OPT_e, options::OPT_s,
+                   options::OPT_t, options::OPT_Z_Flag, options::OPT_r});
 
   if (D.IsUsingLTO(Args))
     AddGoldPlugin(ToolChain, Args, CmdArgs);
 
   AddLinkerInputs(ToolChain, Inputs, Args, CmdArgs);
 
   if (!Args.hasArg(options::OPT_nostdlib) &&
       !Args.hasArg(options::OPT_nodefaultlibs)) {
if (Args.hasArg(options::OPT_ccc_arcmt_check,
     C.addCommand(llvm::make_unique<Command>(JA, *this, Exec, CmdArgs, None));
     return;
   }
 
   // I'm not sure why this particular decomposition exists in gcc, but
   // we follow suite for ease of comparison.
   AddLinkArgs(C, Args, CmdArgs, Inputs);
 
-  Args.AddAllArgs(CmdArgs, options::OPT_d_Flag);
-  Args.AddAllArgs(CmdArgs, options::OPT_s);
-  Args.AddAllArgs(CmdArgs, options::OPT_t);
-  Args.AddAllArgs(CmdArgs, options::OPT_Z_Flag);
-  Args.AddAllArgs(CmdArgs, options::OPT_u_Group);
-  Args.AddLastArg(CmdArgs, options::OPT_e);
-  Args.AddAllArgs(CmdArgs, options::OPT_r);
+  // It seems that the 'e' option is completely ignored for dynamic executables
+  // (the default), and with static executables, the last one wins, as expected.
+  Args.AddAllArgs(CmdArgs, {options::OPT_d_Flag, options::OPT_s, options::OPT_t,
+                            options::OPT_Z_Flag, options::OPT_u_Group,
+                            options::OPT_e, options::OPT_r});
 
   // Forward -ObjC when either -ObjC or -ObjC++ is used, to force loading
   // members of static archive libraries which implement Objective-C classes or
   // categories.
   if (Args.hasArg(options::OPT_ObjC) || Args.hasArg(options::OPT_ObjCXX))
     CmdArgs.push_back("-ObjC");
 
   CmdArgs.push_back("-o");
if (!Args.hasArg(options::OPT_shared)) {
       CmdArgs.push_back(Args.MakeArgString(GCCLibPath + "crtbegin.o"));
     }
     if (getToolChain().getDriver().CCCIsCXX())
       CmdArgs.push_back(Args.MakeArgString(LibPath + "cxa_finalize.o"));
   }
 
   CmdArgs.push_back(Args.MakeArgString("-L" + GCCLibPath));
 
-  Args.AddAllArgs(CmdArgs, options::OPT_L);
-  Args.AddAllArgs(CmdArgs, options::OPT_T_Group);
-  Args.AddAllArgs(CmdArgs, options::OPT_e);
-  Args.AddAllArgs(CmdArgs, options::OPT_r);
+  Args.AddAllArgs(CmdArgs, {options::OPT_L, options::OPT_T_Group,
+                            options::OPT_e, options::OPT_r});
 
   AddLinkerInputs(getToolChain(), Inputs, Args, CmdArgs);
 
   if (!Args.hasArg(options::OPT_nostdlib) &&
       !Args.hasArg(options::OPT_nodefaultlibs)) {
     if (getToolChain().getDriver().CCCIsCXX())
       getToolChain().AddCXXStdlibLibArgs(Args, CmdArgs);
     CmdArgs.push_back("-lgcc_s");
void openbsd::Linker::ConstructJob(Compilation &C, const JobAction &JA,
   }
 
   std::string Triple = getToolChain().getTripleString();
   if (Triple.substr(0, 6) == "x86_64")
     Triple.replace(0, 6, "amd64");
   CmdArgs.push_back(
       Args.MakeArgString("-L/usr/lib/gcc-lib/" + Triple + "/4.2.1"));
 
-  Args.AddAllArgs(CmdArgs, options::OPT_L);
-  Args.AddAllArgs(CmdArgs, options::OPT_T_Group);
-  Args.AddAllArgs(CmdArgs, options::OPT_e);
-  Args.AddAllArgs(CmdArgs, options::OPT_s);
-  Args.AddAllArgs(CmdArgs, options::OPT_t);
-  Args.AddAllArgs(CmdArgs, options::OPT_Z_Flag);
-  Args.AddAllArgs(CmdArgs, options::OPT_r);
+  Args.AddAllArgs(CmdArgs, {options::OPT_L, options::OPT_T_Group,
+                            options::OPT_e, options::OPT_s, options::OPT_t,
+                            options::OPT_Z_Flag, options::OPT_r});
 
   AddLinkerInputs(getToolChain(), Inputs, Args, CmdArgs);
 
   if (!Args.hasArg(options::OPT_nostdlib) &&
       !Args.hasArg(options::OPT_nodefaultlibs)) {
     if (D.CCCIsCXX()) {
       getToolChain().AddCXXStdlibLibArgs(Args, CmdArgs);
       if (Args.hasArg(options::OPT_pg))
if (!Args.hasArg(options::OPT_shared)) {
       CmdArgs.push_back(
           Args.MakeArgString(getToolChain().GetFilePath("crtbegin.o")));
     } else {
       CmdArgs.push_back(
           Args.MakeArgString(getToolChain().GetFilePath("crtbeginS.o")));
     }
   }
 
-  Args.AddAllArgs(CmdArgs, options::OPT_L);
-  Args.AddAllArgs(CmdArgs, options::OPT_T_Group);
-  Args.AddAllArgs(CmdArgs, options::OPT_e);
+  Args.AddAllArgs(CmdArgs,
+                  {options::OPT_L, options::OPT_T_Group, options::OPT_e});
 
   AddLinkerInputs(getToolChain(), Inputs, Args, CmdArgs);
 
   if (!Args.hasArg(options::OPT_nostdlib) &&
       !Args.hasArg(options::OPT_nodefaultlibs)) {
     if (D.CCCIsCXX()) {
       getToolChain().AddCXXStdlibLibArgs(Args, CmdArgs);
       if (Args.hasArg(options::OPT_pg))
if (!Args.hasArg(options::OPT_nostdlib) &&
       !Args.hasArg(options::OPT_nostartfiles)) {
     CmdArgs.push_back(Args.MakeArgString(getToolChain().GetFilePath("crt1.o")));
     CmdArgs.push_back(Args.MakeArgString(getToolChain().GetFilePath("crti.o")));
     CmdArgs.push_back(
         Args.MakeArgString(getToolChain().GetFilePath("crtbegin.o")));
     CmdArgs.push_back(Args.MakeArgString(getToolChain().GetFilePath("crtn.o")));
   }
 
-  Args.AddAllArgs(CmdArgs, options::OPT_L);
-  Args.AddAllArgs(CmdArgs, options::OPT_T_Group);
-  Args.AddAllArgs(CmdArgs, options::OPT_e);
+  Args.AddAllArgs(CmdArgs,
+                  {options::OPT_L, options::OPT_T_Group, options::OPT_e});
 
   AddLinkerInputs(getToolChain(), Inputs, Args, CmdArgs);
 
   addProfileRT(getToolChain(), Args, CmdArgs);
 
   if (!Args.hasArg(options::OPT_nostdlib) &&
       !Args.hasArg(options::OPT_nodefaultlibs)) {
     if (D.CCCIsCXX()) {
if (!Args.hasArg(options::OPT_nostdlib) &&
     if (Args.hasArg(options::OPT_shared) || Args.hasArg(options::OPT_pie))
       CmdArgs.push_back(
           Args.MakeArgString(getToolChain().GetFilePath("crtbeginS.o")));
     else
       CmdArgs.push_back(
           Args.MakeArgString(getToolChain().GetFilePath("crtbegin.o")));
   }
 
-  Args.AddAllArgs(CmdArgs, options::OPT_L);
-  Args.AddAllArgs(CmdArgs, options::OPT_T_Group);
-  Args.AddAllArgs(CmdArgs, options::OPT_e);
+  Args.AddAllArgs(CmdArgs,
+                  {options::OPT_L, options::OPT_T_Group, options::OPT_e});
 
   AddLinkerInputs(getToolChain(), Inputs, Args, CmdArgs);
 
   if (!Args.hasArg(options::OPT_nostdlib) &&
       !Args.hasArg(options::OPT_nodefaultlibs)) {
     // FIXME: GCC passes on -lgcc, -lgcc_pic and a whole lot of
     //         rpaths
     if (UseGCC47)
std::unique_ptr<Command> visualstudio::Compiler::GetCommand(
   CmdArgs.push_back("/nologo");
   CmdArgs.push_back("/c");  // Compile only.
   CmdArgs.push_back("/W0"); // No warnings.
 
   // The goal is to be able to invoke this tool correctly based on
   // any flag accepted by clang-cl.
 
   // These are spelled the same way in clang and cl.exe,.
-  Args.AddAllArgs(CmdArgs, options::OPT_D, options::OPT_U);
-  Args.AddAllArgs(CmdArgs, options::OPT_I);
+  Args.AddAllArgs(CmdArgs, {options::OPT_D, options::OPT_U, options::OPT_I});
 
   // Optimization level.
   if (Arg *A = Args.getLastArg(options::OPT_fbuiltin, options::OPT_fno_builtin))
     CmdArgs.push_back(A->getOption().getID() == options::OPT_fbuiltin ? "/Oi"
                                                                       : "/Oi-");
   if (Arg *A = Args.getLastArg(options::OPT_O, options::OPT_O0)) {
     if (A->getOption().getID() == options::OPT_O0) {
       CmdArgs.push_back("/Od");
void tools::SHAVE::Compiler::ConstructJob(Compilation &C, const JobAction &JA,
 
   ArgStringList CmdArgs;
 
   assert(Inputs.size() == 1);
   const InputInfo &II = Inputs[0];
   assert(II.getType() == types::TY_C || II.getType() == types::TY_CXX);
   assert(Output.getType() == types::TY_PP_Asm); // Require preprocessed asm.
 
-  // Append all -I, -iquote, -isystem paths.
-  Args.AddAllArgs(CmdArgs, options::OPT_clang_i_Group);
+  // Append all -I, -iquote, -isystem paths, and defines/undefines.
   // These are spelled the same way in clang and moviCompile.
-  Args.AddAllArgs(CmdArgs, options::OPT_D, options::OPT_U);
+  Args.AddAllArgs(CmdArgs,
+                  {options::OPT_clang_i_Group, options::OPT_D, options::OPT_U});
 
   CmdArgs.push_back("-DMYRIAD2");
   CmdArgs.push_back("-mcpu=myriad2");
   CmdArgs.push_back("-S");
 
   // Any -O option passes through without translation. What about -Ofast ?
   if (Arg *A = Args.getLastArg(options::OPT_O_Group))
     A->render(Args, CmdArgs);

cfe/trunk/test/Driver/cl-response-file.c

 // Test that we use the Windows tokenizer for clang-cl response files. The
 // trailing backslash before the space should be interpreted as a literal
 // backslash. PR23709
 
 
 
 // RUN: echo '/I%S\Inputs\cl-response-file\ /DFOO=2' > %t.rsp
 // RUN: %clang_cl /c -### @%t.rsp -- %s 2>&1 | FileCheck %s
 
-// CHECK: "-D" "FOO=2" "-I" "{{.*}}\\Inputs\\cl-response-file\\"
+// CHECK: "-I" "{{.*}}\\Inputs\\cl-response-file\\" "-D" "FOO=2"

cfe/trunk/test/Driver/hexagon-toolchain-elf.c

 // CHECK022: "{{.*}}/hexagon/lib/v4/crt0_standalone.o"
 // CHECK022: "{{.*}}/hexagon/lib/v4/crt0.o"
 // CHECK022: "{{.*}}/hexagon/lib/v4/init.o"
 // CHECK022: "-L{{.*}}/lib/gcc/hexagon/4.4.0/v4"
 // CHECK022: "-L{{.*}}/lib/gcc/hexagon/4.4.0"
 // CHECK022: "-L{{.*}}/lib/gcc"
 // CHECK022: "-L{{.*}}/hexagon/lib/v4"
 // CHECK022: "-L{{.*}}/hexagon/lib"
-// CHECK022: "-Tbss" "0xdead" "-Tdata" "0xbeef" "-Ttext" "0xcafe"
 // CHECK022: "-s"
+// CHECK022: "-Tbss" "0xdead" "-Tdata" "0xbeef" "-Ttext" "0xcafe"
 // CHECK022: "-t"
 // CHECK022: "-u" "Foo" "-undefined" "Bar"
 // CHECK022: "{{[^"]+}}.o"
 // CHECK022: "-lstdc++" "-lm"
 // CHECK022: "--start-group" "-lstandalone" "-lc" "-lgcc" "--end-group"
 // CHECK022: "{{.*}}/hexagon/lib/v4/fini.o"
 
 // -----------------------------------------------------------------------------

cfe/trunk/test/Driver/hexagon-toolchain.c

 // CHECK022: "{{.*}}/hexagon/lib/v4/crt0_standalone.o"
 // CHECK022: "{{.*}}/hexagon/lib/v4/crt0.o"
 // CHECK022: "{{.*}}/hexagon/lib/v4/init.o"
 // CHECK022: "-L{{.*}}/lib/gcc/hexagon/4.4.0/v4"
 // CHECK022: "-L{{.*}}/lib/gcc/hexagon/4.4.0"
 // CHECK022: "-L{{.*}}/lib/gcc"
 // CHECK022: "-L{{.*}}/hexagon/lib/v4"
 // CHECK022: "-L{{.*}}/hexagon/lib"
-// CHECK022: "-Tbss" "0xdead" "-Tdata" "0xbeef" "-Ttext" "0xcafe"
 // CHECK022: "-s"
+// CHECK022: "-Tbss" "0xdead" "-Tdata" "0xbeef" "-Ttext" "0xcafe"
 // CHECK022: "-t"
 // CHECK022: "-u" "Foo" "-undefined" "Bar"
 // CHECK022: "{{[^"]+}}.o"
 // CHECK022: "-lstdc++" "-lm"
 // CHECK022: "--start-group" "-lstandalone" "-lc" "-lgcc" "--end-group"
 // CHECK022: "{{.*}}/hexagon/lib/v4/fini.o"
 
 // -----------------------------------------------------------------------------