[PATCH] D11581: [SHAVE] Pass all -f, -g, -O, -W options through directly to moviCompile.

[James Y Knight]

jyknight added inline comments.

================
Comment at: lib/Driver/Tools.cpp:9482-9488
@@ -9481,13 +9481,9 @@
 
-  // Any -O option passes through without translation. What about -Ofast ?
-  if (Arg *A = Args.getLastArg(options::OPT_O_Group))
-    A->render(Args, CmdArgs);
-
-  if (Args.hasFlag(options::OPT_ffunction_sections,
-                   options::OPT_fno_function_sections)) {
-    CmdArgs.push_back("-ffunction-sections");
-  }
-  if (Args.hasArg(options::OPT_fno_inline_functions))
-    CmdArgs.push_back("-fno-inline-functions");
-
+  Args.AddAllArgs(CmdArgs,
+                  options::OPT_f_Group,
+                  options::OPT_f_clang_Group,
+                  options::OPT_g_Group);
+  Args.AddAllArgs(CmdArgs,
+                  options::OPT_O_Group,
+                  options::OPT_W_Group);
   CmdArgs.push_back("-fno-exceptions"); // Always do this even if unspecified.
----------------
chandlerc wrote:
> How worried are you about eventual incompatibilities between the flags in these groups for the driver and for the underlying SHAVE compiler? I'm moderately worried...
> 
> How many flags are we talking about here? Also, the -W flags worry me less than the rest.
> 
> Finally, why two AddAllArgs?
> How worried are you about eventual incompatibilities between the flags in these groups for the driver and for the underlying SHAVE compiler? I'm moderately worried...

Well, what bad would happen? The thing I'd expect is to have an arg that's been removed from the driver but is still supported by the underlying compiler, or, is not yet present in the underlying compiler but is supported in the driver. Neither of those seem terribly problematic -- you'll get an error from one layer or the other, which is basically the correct thing to have happen.

I think the only actual bad case is if a given flag changes between having a separate value to not, or vice versa, and that seems unlikely enough to come up that it can be ignored as a problem.

> why two AddAllArgs?

Because AddAllArgs can take 3 args, and only 3 args, and that's true all the way down through "class arg_iterator", which has 3 members, "OptSpecifier Id0, Id1, Id2", rather than, say, a SmallVector<OptSpecifier, 3> Ids. Yuk.




http://reviews.llvm.org/D11581