[PATCH] D110663: [Driver] Support Debian multiarch style lib/clang/14.0.0/x86_64-linux-gnu runtime path and include/x86_64-linux-gnu/c++/v1 libc++ path

[Fangrui Song via Phabricator via cfe-commits]

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