michaelplatings added inline comments.
================
Comment at: clang/include/clang/Driver/Multilib.h:41
+ flag_set Flags;
+ arg_list PrintArgs;
----------------
phosek wrote:
> This is just a suggestion, but GCC documentation refers to these as either
> "options" or "switches", not "args" so I think it might be helpful to use the
> same nomenclature. This would also avoid confusion since the term "args" is
> used extensively throughout the driver but refers to input arguments.
OK, I've gone with "options" just because `switch_list` would be an unintuitive
name.
================
Comment at: clang/lib/Driver/MultilibBuilder.cpp:90-93
+ for (StringRef Flag : Flags) {
+ if (Flag.front() == '+')
+ Args.push_back(("-" + Flag.substr(1)).str());
+ }
----------------
phosek wrote:
> If I understand this correctly, we might end up with duplicate arguments in
> the case when `Flags` contains duplicate elements. Is that desirable?
> Wouldn't it be better to construct the list of arguments from the set of fags
> inside `Multilib`?
Yes, you can have duplicate options if you write code to do that. The Multilib
class has never previously had any functionality to remove duplicate options so
I'd rather not change that. Previously if you wrote code that added the same
option twice then `clang -print-multi-lib` would reflect that. This behaviour
remains unchanged.
No, you can't construct the list of options from the set of flags inside
`Multilib` because `Multilib`'s flags are not command line options. (They will
typically look similar to command line options but there's no requirement for
that to be the case). I've added comments here and for the flags() method to
hopefully make this clearer. Hopefully the docs in D143587 will also make
things clearer for anyone else trying to reason about the multilib system.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D142905/new/
https://reviews.llvm.org/D142905
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
