[PATCH] D41316: [libcxx] Allow random_device to be built optionally

Eric Fiselier via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Feb 9 18:10:23 PST 2018


EricWF added a comment.

I think the direction here is OK. Although I really hate allowing bits of libc++ to be "optional". In this case it should be OK, but often it makes the library trickier to maintain -- and these configurations often go untested between releases.



================
Comment at: CMakeLists.txt:74
 option(LIBCXX_INCLUDE_TESTS "Build the libc++ tests." ${LLVM_INCLUDE_TESTS})
+option(LIBCXX_ENABLE_RANDOM_DEVICE "Build random_device class" On)
 
----------------
nit: `ON`


================
Comment at: test/std/experimental/filesystem/lit.local.cfg:5
+
+# filesystem_test_helper uses random_device to generate random path.
+if 'libcpp-has-no-random-device' in config.available_features:
----------------
I would rather keep these tests running using a different source for the random seed if possible.


================
Comment at: test/std/numerics/rand/rand.device/lit.local.cfg:1
+# Disable all of the random device tests if the correct feature is not available.
+if 'libcpp-has-no-random-device' in config.available_features:
----------------
There are only 3 tests under this directory. I would rather mark each one explicitly with `// UNSUPPORTED: libcpp-has-no-random-device`


Repository:
  rCXX libc++

https://reviews.llvm.org/D41316





More information about the cfe-commits mailing list