[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

[Petr Hosek via Phabricator via cfe-commits]

phosek added a subscriber: hvdijk.
phosek added a comment.

In D110663#3270509 <https://reviews.llvm.org/D110663#3270509>, @MaskRay wrote:

> Ping @phosek

I apologize about the belated response, but I wanted to test this change first given the potentially large blast radius.

To clarify, when I talk Fuchsia toolchain, I don't just mean toolchain targeting Fuchsia, but a Clang toolchain distribution our team manages that's also used by a number of other teams such Dart, Flutter, Pigweed to build code for variety of platforms including Linux.

In my testing, I found that these users aren't currently using a single uniform target triple spelling, I've seen both `${arch}-linux-gnu` and `${arch}-unknown-linux-gnu` used pretty liberally. So if we were to land this change today, it would break many of these users. We could clean up and unify all of these uses, but it's going to take some effort because it spans lots of projects. I'm also worried that there might be other Clang users in a similar situation who are not included on this change.

That's why I think there should be an RFC sent to cfe-dev, to both reach an agreement on whether this is a direction everyone agrees with, but also to raise the awareness. There's also a question whether this should be rolled out in a more gradual fashion (for example by starting with a warning first).

Furthermore, I'm also not sure about the motivation behind this change. The change summary mentions D109837 <https://reviews.llvm.org/D109837>, but @hvdijk said that this is not a blocker. Is there another reason for doing this?



================
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
----------------
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".


================
Comment at: clang/lib/Driver/Driver.cpp:1203-1206
+  if (Normalized.isOSFuchsia())
+    TargetTriple = Normalized.str();
+  else
+    TargetTriple = Unnormalized.str();
----------------
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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110663/new/

https://reviews.llvm.org/D110663