MaskRay added inline comments.
================
Comment at: clang/lib/Driver/Driver.cpp:428
+ const Driver &D, StringRef TargetTriple, const ArgList &Args,
+ llvm::Triple *UnnormalizedTriple = nullptr, StringRef DarwinArchName = "")
{
// FIXME: Already done in Compilation *Driver::BuildCompilation
----------------
phosek wrote:
> I find the name `UnnormalizedTriple` a bit confusing as it may imply that
> there's some "un-normalization" process involved. I'd prefer using a
> `TargetTriple` to refer to "triple given by the user (via the `--target=`
> option)" and `NormalizedTriple` as "normalized triple".
`llvm::Triple Target(llvm::Triple::normalize(TargetTriple));` refers to the
normalized triple is references in many places. Having a `TargetTriple` may
cause confusion.
I also want to avoid renaming `Target` (referenced in too many places) to
minimize the diff.
================
Comment at: clang/lib/Driver/Driver.cpp:1203-1206
+ if (Normalized.isOSFuchsia())
+ TargetTriple = Normalized.str();
+ else
+ TargetTriple = Unnormalized.str();
----------------
phosek wrote:
> I really think this logic belongs to `clang::driver::toolchains::Fuchsia`.
> Moving it there is going to require some changes to how toolchains are
> instantiated, and should thus be done as a separate change, but doing so
> could also help us cleanup other special case logic like that exists in
> `Driver` like
> https://github.com/llvm/llvm-project/blob/8ba9c794feb30cd969b9776c39873def10c51bff/clang/lib/Driver/Driver.cpp#L576.
I agree. The change seems involved so I do not do it here...
Moving `toolchains::MinGW::fixTripleArch(D, Target, Args);` to the
toolchain-specific file will indeed be nice.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D110663/new/
https://reviews.llvm.org/D110663
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
