GMNGeoffrey accepted this revision.
GMNGeoffrey added inline comments.
This revision is now accepted and ready to land.


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


================
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*",
----------------
Probably good to note here that these are also disabled in the CMake build


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


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

https://reviews.llvm.org/D112399

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to