[PATCH] D65108: Reland "driver: Don't warn about assembler flags being unused when not assembling"

[Nico Weber via Phabricator via cfe-commits]

thakis marked an inline comment as done.
thakis added inline comments.


================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:3564
+    ArgStringList DummyArgs;
+    CollectArgsForIntegratedAssembler(C, Args, DummyArgs, D,
+                                      TC.useIntegratedAs());
----------------
thakis wrote:
> rnk wrote:
> > I think it would be better to use ClaimAllArgs here. If you look around in this code, there's a fair amount of stuff like this:
> > ```
> >   if (KernelOrKext) {
> >     // -mkernel and -fapple-kext imply no exceptions, so claim exception related
> >     // arguments now to avoid warnings about unused arguments.
> >     Args.ClaimAllArgs(options::OPT_fexceptions);
> >     Args.ClaimAllArgs(options::OPT_fno_exceptions);
> >     Args.ClaimAllArgs(options::OPT_fobjc_exceptions);
> >     Args.ClaimAllArgs(options::OPT_fno_objc_exceptions);
> >     Args.ClaimAllArgs(options::OPT_fcxx_exceptions);
> >     Args.ClaimAllArgs(options::OPT_fno_cxx_exceptions);
> >     return;
> >   }
> > ```
> > It repeats the knowledge of which flags are passed to the assembler (looks like 5), but it is consistent with what's already done.
> I think that's worse for this use case here: In addition of having to duplicate the flags, the flag parsing error output (for integrated as) is then again dependent on if we actually run the assembler, which is the same kind of inconsistency that this change is fixing.
I uploaded a patch for the approach you suggested to https://reviews.llvm.org/D65233 . It's ~20 (non-whitespace) lines of code, while this patch is ~10 lines (it looks like more because phab doesn't highlight whitespace-only changes well.)

I like this patch more, but let me know which one you'd like me to land.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65108/new/

https://reviews.llvm.org/D65108