[PATCH] D54379: Add Hurd toolchain support to Clang

[Samuel Thibault via Phabricator via cfe-commits]

sthibaul marked 9 inline comments as done.
sthibaul added inline comments.


================
Comment at: lib/Basic/Targets/OSTargets.h:283
+    Builder.defineMacro("__GLIBC__");
+    Builder.defineMacro("__ELF__");
+    if (Opts.POSIXThreads)
----------------
kristina wrote:
> `__MACH__` and `__HURD__` seem appropriate? Apple Mach (Darwin/XNU) uses `__MACH__` with `__APPLE__`, Hurd should probably follow a similar convention, I don't think there are many places aside from XNU build where `__MACH__` is used on its own.
There is actually no `__HURD__` macro, it's the `__GNU__` macro which has that role. `__MACH__` should however  be there too indeed, as well as `__gnu_hurd__` similarly to Linux' `__gnu_linux__`.


================
Comment at: lib/Driver/ToolChains/Hurd.cpp:78
+
+  return std::string();
+}
----------------
kristina wrote:
> I'm not quite sure I like this. Also early return should be for the "bad" case, not for the good case, at least IMO, this is not a huge issue but I'll see what others say. I think this may just be subjective.
Well, this is inspired from clang/lib/Driver/ToolChains/Linux.cpp, which additionally has some gcc tests, which I'll include in a later patch. That argues for using this way of doing the test since that is how it will be in the end.


Repository:
  rC Clang

https://reviews.llvm.org/D54379