<div dir="ltr">LGTM</div><div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, Jul 25, 2013 at 3:34 PM, Hans Wennborg <span dir="ltr"><<a href="mailto:hans@chromium.org" target="_blank">hans@chromium.org</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi rnk,<br>
<br>
They seem to have the same implications, and this makes for one less flag to worry about.<br>
<br>
<a href="http://llvm-reviews.chandlerc.com/D1219" target="_blank">http://llvm-reviews.chandlerc.com/D1219</a><br>
<br>
Files:<br>
  include/clang/Driver/Options.h<br>
  include/clang/Driver/Options.td<br>
  lib/Driver/Tools.cpp<br>
<br>
Index: include/clang/Driver/Options.h<br>
===================================================================<br>
--- include/clang/Driver/Options.h<br>
+++ include/clang/Driver/Options.h<br>
@@ -26,10 +26,9 @@<br>
   DriverOption = (1 << 4),<br>
   LinkerInput = (1 << 5),<br>
   NoArgumentUnused = (1 << 6),<br>
-  NoForward = (1 << 7),<br>
-  Unsupported = (1 << 8),<br>
-  CC1Option = (1 << 9),<br>
-  NoDriverOption = (1 << 10)<br>
+  Unsupported = (1 << 7),<br>
+  CC1Option = (1 << 8),<br>
+  NoDriverOption = (1 << 9)<br>
 };<br>
<br>
 enum ID {<br>
Index: include/clang/Driver/Options.td<br>
===================================================================<br>
--- include/clang/Driver/Options.td<br>
+++ include/clang/Driver/Options.td<br>
@@ -18,7 +18,7 @@<br>
 // Flags<br>
<br>
 // DriverOption - The option is a "driver" option, and should not be forwarded<br>
-// to gcc.<br>
+// to other tools.<br>
 def DriverOption : OptionFlag;<br>
<br>
 // LinkerInput - The option is a linker input.<br>
@@ -33,9 +33,6 @@<br>
 // lines that use it.<br>
 def Unsupported : OptionFlag;<br>
<br>
-// NoForward - The option should not be implicitly forwarded to other tools.<br>
-def NoForward : OptionFlag;<br>
-<br>
 // CC1Option - This option should be accepted by clang -cc1.<br>
 def CC1Option : OptionFlag;<br>
<br>
@@ -252,7 +249,7 @@<br>
   HelpText<"Pass <arg> to the assembler">, MetaVarName<"<arg>">;<br>
 def Xclang : Separate<["-"], "Xclang">,<br>
   HelpText<"Pass <arg> to the clang compiler">, MetaVarName<"<arg>">,<br>
-  Flags<[NoForward]>;<br>
+  Flags<[DriverOption]>;<br>
 def Xlinker : Separate<["-"], "Xlinker">, Flags<[LinkerInput, RenderAsInput]>,<br>
   HelpText<"Pass <arg> to the linker">, MetaVarName<"<arg>">;<br>
 def Xpreprocessor : Separate<["-"], "Xpreprocessor">,<br>
@@ -340,7 +337,8 @@<br>
 def fasynchronous_unwind_tables : Flag<["-"], "fasynchronous-unwind-tables">, Group<f_Group>;<br>
<br>
 def fautolink : Flag <["-"], "fautolink">, Group<f_Group>;<br>
-def fno_autolink : Flag <["-"], "fno-autolink">, Group<f_Group>, Flags<[NoForward, CC1Option]>,<br>
+def fno_autolink : Flag <["-"], "fno-autolink">, Group<f_Group>,<br>
+  Flags<[DriverOption, CC1Option]>,<br>
   HelpText<"Disable generation of linker directives for automatic library linking">;<br>
<br>
 def fblocks : Flag<["-"], "fblocks">, Group<f_Group>, Flags<[CC1Option]>,<br>
@@ -378,7 +376,8 @@<br>
 def fcreate_profile : Flag<["-"], "fcreate-profile">, Group<f_Group>;<br>
 def fcxx_exceptions: Flag<["-"], "fcxx-exceptions">, Group<f_Group>,<br>
   HelpText<"Enable C++ exceptions">, Flags<[CC1Option]>;<br>
-def fcxx_modules : Flag <["-"], "fcxx-modules">, Group<f_Group>, Flags<[NoForward]>;<br>
+def fcxx_modules : Flag <["-"], "fcxx-modules">, Group<f_Group>,<br>
+  Flags<[DriverOption]>;<br>
 def fdebug_pass_arguments : Flag<["-"], "fdebug-pass-arguments">, Group<f_Group>;<br>
 def fdebug_pass_structure : Flag<["-"], "fdebug-pass-structure">, Group<f_Group>;<br>
 def fdiagnostics_fixit_info : Flag<["-"], "fdiagnostics-fixit-info">, Group<f_clang_Group>;<br>
@@ -534,15 +533,16 @@<br>
   HelpText<"Parse templated function definitions at the end of the "<br>
            "translation unit ">,  Flags<[CC1Option]>;<br>
 def fmodules_cache_path : Joined<["-"], "fmodules-cache-path=">, Group<i_Group>,<br>
-  Flags<[NoForward,CC1Option]>, MetaVarName<"<directory>">,<br>
+  Flags<[DriverOption, CC1Option]>, MetaVarName<"<directory>">,<br>
   HelpText<"Specify the module cache path">;<br>
 def fmodules_prune_interval : Joined<["-"], "fmodules-prune-interval=">, Group<i_Group>,<br>
   Flags<[CC1Option]>, MetaVarName<"<seconds>">,<br>
   HelpText<"Specify the interval (in seconds) between attempts to prune the module cache">;<br>
 def fmodules_prune_after : Joined<["-"], "fmodules-prune-after=">, Group<i_Group>,<br>
   Flags<[CC1Option]>, MetaVarName<"<seconds>">,<br>
   HelpText<"Specify the interval (in seconds) after which a module file will be considered unused">;<br>
-def fmodules : Flag <["-"], "fmodules">, Group<f_Group>, Flags<[NoForward,CC1Option]>,<br>
+def fmodules : Flag <["-"], "fmodules">, Group<f_Group>,<br>
+  Flags<[DriverOption, CC1Option]>,<br>
   HelpText<"Enable the 'modules' language feature">;<br>
 def fmodules_ignore_macro : Joined<["-"], "fmodules-ignore-macro=">, Group<f_Group>, Flags<[CC1Option]>,<br>
   HelpText<"Ignore the definition of the given macro when building and loading modules">;<br>
@@ -578,7 +578,8 @@<br>
   Flags<[CC1Option]>,<br>
   HelpText<"Disable creation of CodeFoundation-type constant strings">;<br>
 def fno_cxx_exceptions: Flag<["-"], "fno-cxx-exceptions">, Group<f_Group>;<br>
-def fno_cxx_modules : Flag <["-"], "fno-cxx-modules">, Group<f_Group>, Flags<[NoForward]>;<br>
+def fno_cxx_modules : Flag <["-"], "fno-cxx-modules">, Group<f_Group>,<br>
+  Flags<[DriverOption]>;<br>
 def fno_diagnostics_fixit_info : Flag<["-"], "fno-diagnostics-fixit-info">, Group<f_Group>,<br>
   Flags<[CC1Option]>, HelpText<"Do not include fixit information in diagnostics">;<br>
 def fno_diagnostics_show_name : Flag<["-"], "fno-diagnostics-show-name">, Group<f_Group>;<br>
@@ -601,7 +602,8 @@<br>
   HelpText<"Do not limit debug information produced to reduce size of debug binary">;<br>
 def fno_merge_all_constants : Flag<["-"], "fno-merge-all-constants">, Group<f_Group>,<br>
     Flags<[CC1Option]>, HelpText<"Disallow merging of constants">;<br>
-def fno_modules : Flag <["-"], "fno-modules">, Group<f_Group>, Flags<[NoForward]>;<br>
+def fno_modules : Flag <["-"], "fno-modules">, Group<f_Group>,<br>
+  Flags<[DriverOption]>;<br>
 def fno_ms_extensions : Flag<["-"], "fno-ms-extensions">, Group<f_Group>;<br>
 def fno_ms_compatibility : Flag<["-"], "fno-ms-compatibility">, Group<f_Group>;<br>
 def fno_delayed_template_parsing : Flag<["-"], "fno-delayed-template-parsing">, Group<f_Group>;<br>
@@ -906,7 +908,8 @@<br>
 def mios_version_min_EQ : Joined<["-"], "mios-version-min=">, Alias<miphoneos_version_min_EQ>;<br>
 def mios_simulator_version_min_EQ : Joined<["-"], "mios-simulator-version-min=">, Group<m_Group>;<br>
 def mkernel : Flag<["-"], "mkernel">, Group<m_Group>;<br>
-def mlinker_version_EQ : Joined<["-"], "mlinker-version=">, Flags<[NoForward]>;<br>
+def mlinker_version_EQ : Joined<["-"], "mlinker-version=">,<br>
+  Flags<[DriverOption]>;<br>
 def mllvm : Separate<["-"], "mllvm">, Flags<[CC1Option]>,<br>
   HelpText<"Additional arguments to forward to LLVM's option processing">;<br>
 def mmacosx_version_min_EQ : Joined<["-"], "mmacosx-version-min=">, Group<m_Group>;<br>
Index: lib/Driver/Tools.cpp<br>
===================================================================<br>
--- lib/Driver/Tools.cpp<br>
+++ lib/Driver/Tools.cpp<br>
@@ -228,7 +228,6 @@<br>
   // Don't forward inputs from the original command line.  They are added from<br>
   // InputInfoList.<br>
   return O.getKind() != Option::InputClass &&<br>
-         !O.hasFlag(options::NoForward) &&<br>
          !O.hasFlag(options::DriverOption) &&<br>
          !O.hasFlag(options::LinkerInput);<br>
 }<br>
</blockquote></div><br></div>