[PATCH] D65108: Reland "driver: Don't warn about assembler flags being unused when not assembling"
Nico Weber via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Jul 22 16:58:35 PDT 2019
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
More information about the cfe-commits
mailing list