egorzhdan marked 2 inline comments as done. egorzhdan added inline comments.
================ Comment at: clang/lib/Driver/ToolChain.cpp:917 +/*static*/ std::string ToolChain::concat(const std::string &Path, + const Twine &A, const Twine &B, ---------------- MaskRay wrote: > I think the first argument of `concat` should be `StringRef`, then we can > avoid changing the signature of `getMultiarchTriple` You're right, thanks! ================ Comment at: clang/test/Driver/linux-header-search.cpp:76 + +// Test Linux with libstdc++. +// RUN: %clang -### %s -fsyntax-only 2>&1 \ ---------------- MaskRay wrote: > I suspect this test does not add more coverage than linux-cross.cpp. > > To test --sysroot with a trailing `/`, just modify the previous > `CHECK-BASIC-LIBCXXV2` by appending a `/` `CHECK-BASIC-LIBCXXV2 ` is not quite the same: I'd like to test the libstdc++ detection logic (e.g. `GCCInstallationDetector`) which doesn't get invoked for libc++. I removed the unnecessary test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126289/new/ https://reviews.llvm.org/D126289 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
