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());
----------------
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.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65108/new/
https://reviews.llvm.org/D65108
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
