[libcxx-commits] [PATCH] D97539: [libcxx] Implement semaphores for windows

Fri Mar 5 00:43:37 PST 2021

mstorsjo added a comment.

In D97539#2604919 <https://reviews.llvm.org/D97539#2604919>, @ldionne wrote:

> I've said it before, but I would be happier if we had a tester setup for windows. @rnk @amccarth Do any of you think that would be something feasible? The BuildKite setup is really easy to interface with.

Yeah, that'd be very much appreciated. With the latest couple patches and the updated instructions in D97166 <https://reviews.llvm.org/D97166>, it now runs pretty well out of the box overall. Continuous testing is sorely needed though, given the recent handful of breakages in MSVC configurations and/or when running tests overall. With current top of tree, there's still around 50 or so failed tests in a full `check-cxx`, but if someone would set up a buildbot, we could slap XFAIL on those, to be able to guard against further regressions and track what's left if things get cleaned up further.

================
Comment at: libcxx/include/__threading_support:153
+#if defined(_LIBCPP_HAS_THREAD_API_WIN32)
+# define _LIBCPP_SEMAPHORE_MAX (numeric_limits<long>::max())
+#endif
----------------
ldionne wrote:
> Let's use `::std::numeric_limits<long>::max()`, just as good hygiene for a macro.
Ok, will do. FWIW I copied the current form from the `<semaphore>` header, from the `_LIBCPP_NO_NATIVE_SEMAPHORES` case (which is the one used on e.g. apple platforms afaik), which also lacks the `::std::` prefix and the explicit `<limits>` include.

================
Comment at: libcxx/include/__threading_support:154
+# define _LIBCPP_SEMAPHORE_MAX (numeric_limits<long>::max())
+#endif

----------------
Quuxplusone wrote:
> Does this file actually `#include <limits>`?
> Also: This is a sneaky way to prevent people from using the macro outside of `namespace std`. 😛
It doesn't include `<limits>` yet, I can add that before pushing.

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

https://reviews.llvm.org/D97539