[PATCH] D112399: Get Bazel building `//clang` on Windows with clang-cl.

[Chandler Carruth via Phabricator via cfe-commits]

chandlerc added a comment.

Thanks, making suggested changes and then landing!



================
Comment at: utils/bazel/llvm-project-overlay/clang/BUILD.bazel:364
+            # Clang-specific define on non-Windows platforms.
+            "CLANG_HAVE_RLIMITS=1",
+        ],
----------------
GMNGeoffrey wrote:
> Seems like this should be in clang/config.h?
Ah, yeah. Might as well.


================
Comment at: utils/bazel/llvm-project-overlay/clang/unittests/BUILD.bazel:485
+        "@bazel_tools//src/conditions:windows": [
+            # Need to disable the VFS tests that don't use Windows friendly paths.
+            "--gtest_filter=-*VirtualFileOverlay*",
----------------
GMNGeoffrey wrote:
> Probably good to note here that these are also disabled in the CMake build
Good idea, will do.


================
Comment at: utils/bazel/llvm-project-overlay/llvm/include/llvm/Config/config.h:375
+ * matches how the users of this header expect things to work with CMake.
+ * FIXME: We should consider moving other platform defines to use this technique
+ * as well.
----------------
GMNGeoffrey wrote:
> Oh nice, yeah this seems like a reasonable approach. I can't believe we didn't think of this before... The less Bazel goop the better. We're still going to need some way to deal with the things that are more complicated than just platform and some way to handle keeping this up to date. That might push us back to doing it with Bazel. IDK
Well, *all* of Abseil's configuration is now done this way I think... so I've some optimism. Anyways, I'll send subsequent patches when I get some time.


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

https://reviews.llvm.org/D112399