[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