Thank you, build is fine after applying the patch.

In D61756#2003836, @MaskRay wrote:

@rmisth wrote: "Clang should drive the standard, not diverge from it", and we should at least try to push for standardizing this if we think it's worthwhile as an alternative to FILE.

In D61756#1505296, @kristina wrote:

Revised to use llvm::sys::path::filename to avoid issues on Windows hosts.

Hi @kristina, have you followed up with the extension on WG14 or WG21 mailing lists? There is a similar GCC feature request which you may want to comment as well https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77488

In D75373#1900237, @aganea wrote:

The patch did not make sense conceptually. Hurd is not Linux. I think now it makes more sense.

I feel like in this specific case it may be worth splitting this into two patches:

Add some reviewers.

Abandoning in favor of D71916.

For future - Since you seem to put up a lot of (well, almost all) patches related to Hurd support, may I suggest requesting commit access so you can land patches yourself after review?

On second look this seems to be caused by compiler-rt being used as the default runtime, which is not accounted for in that particular test.

I ran check-clang on a x86_64 Ubuntu 18.04 machine (toolchain compiled with compiler-rt as the default CRT and libc++/libc++abi/libunwind_llvm). I think one of the driver tests for multilib stuff may be broken, in which case it should probably be investigated/fixed before this lands. And yes you are right, this test seems to fail with this configuration even without the patch applied. Is this the same test you're referring to (cross-linux.c)?

Sorry for responding late, was away. This seems to be tripping up a regression test:

Added libcxx maintainers, would like one of them to sign off on this. I presume this is NFCI for Linux so it should be covered by existing regression tests?

LGTM, at least following the same reasoning as in rC271692 (http://lists.llvm.org/pipermail/cfe-commits/Week-of-Mon-20160530/160984.html). I presume you want to be able to allow the default to be baked into the driver.

In D69683#1730353, @sthibaul wrote:

gnu_hurd can work as well indeed

Is there a better way to check for a Hurd host than using the __GNU__ preprocessor macro? I'm worried it may cause some confusion otherwise similar to the issue with Hurd triples. It seems that Hurd target defines __gnu_hurd__, which is a lot less ambiguous and probably self-explanatory than __GNU__ (especially considering how often __GNUC__ is defined and given that it has a completely different meaning). Aside from that, LGTM!

Just linking relevant bug for the record: https://bugs.llvm.org/show_bug.cgi?id=43517

This seems to have been causing problems during linking, got hundreds of those:

Thank you for taking the time to fix this properly, Modules+LTO linked successfully.

As trivial as it may seem, this is missing tests, even if it's for a few flags. Also please try to include context with your patches as it makes them much easier to review. If you have some sort of plan for actually bringing Mach-O LLD closer to ld64 feature-wise, I think llvm-dev may be a good place to actually outline your plan for this, which will give everyone a more clear picture of what future changes you have planned in mind.

Experienced the same, updated my test build configuration to always force CLANG_ENABLE_STATIC_ANALYZER to On when building with tests. Maybe it's worth adding a warning about when Clang tests are being built?

In D61975#1519367, @efriedma wrote:

Would it make sense to also add a test explicitly using llvm::hash_combine

Using llvm::hash_combine on what, exactly?

as well as a negative test where hash code comparison fails for a literal and the same literal in a StringRef every time

You mean something like ASSERT_NE(SymName1.data(), SymName2.data());?

In D61634#1517228, @xbolva00 wrote:

I have a question about qsort.. If we provide own implementation of qsort and replace calls to libc's qsort to our qsort, we could fully inline cmp function then. Ideas?

Seems good. Would it make sense to also add a test explicitly using llvm::hash_combine, as well as a negative test where hash code comparison fails for a literal and the same literal in a StringRef every time (if it's possible to do reliably)? Just a random suggestion, probably overly paranoid.

My bad, I didn't check well enough, it seems an unrelated patch made certain tests crash due to asserts. Thanks to @craig.topper for pointing that out.

This seems to be causing multiple performance regressions across several bots in compile time and execution time tests.

Revised to use llvm::sys::path::filename to avoid issues on Windows hosts.

Reverted in rL360842 as Windows bots were failing.

Landing this as discussed on IRC, will try to push it forward with WG14.

@rsmith Ping.

Need @rsmith to bless this as it's introducing a nonstandard extension, however small it may be. The original diff did have a consensus on it, so I didn't really put up a formal RFC on cfe-dev.

Actually I got it wrong, the path is normalized to use regular slashes at that point, so there is no point in handling backslashes in paths at all even with Microsoft extensions.

Fix style, remove unnecessary braces, add missing newline.

Superseded by D61756

Sorry, forgot about this, will make a new diff with just the macro for review later tonight.