[libcxx-commits] [PATCH] D120478: [libcxx] [test] Make filesystem tests not rely on libc++ internals

[Martin Storsjö via Phabricator via libcxx-commits]

mstorsjo added inline comments.


================
Comment at: libcxx/test/support/filesystem_test_helper.h:35
+#ifdef _WIN32
+// If built without -DNOMINMAX, Windows system headers can pollute the
+// namespace with min/max defines that break standard C++.
----------------
ldionne wrote:
> ldionne wrote:
> > Instead, could we guard against those macros by doing something like `(std::numeric_limits<large_file_offset_t>::max)()`?
> > 
> > Otherwise, we could also add `-DNOMINMAX` in the testing config when we're on Windows. I'm not a huge fan of undef'ing those macros here in this file, it seems kind of ad-hoc.
> Yeah, FWIW, I think this is where I made a mistake: https://reviews.llvm.org/D119036#3301636
> 
> I should have left `-DNOMINMAX` in place, at least on Windows.
Yeah I don't mind putting that define back.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120478