Details
- Reviewers
EricWF ldionne mclow.lists zoecarver jfb - Group Reviewers
Restricted Project
Diff Detail
Event Timeline
libcxx/include/numbers | ||
---|---|---|
85 | in that case static_assert will fail even when this template is not instantiated. |
libcxx/test/std/numerics/numbers/numbers.fail.cpp | ||
---|---|---|
19 | this testing is that static assert should fail when other than floating type is used. |
Of course, thanks for working on this. LGTM.
(you'll still need a libc++ maintainer's review)
libcxx/include/numbers | ||
---|---|---|
85 | You're right! I always forget :) |
libcxx/include/numbers | ||
---|---|---|
81 | Given that it's a C++20 addition, I think that you don't need to use _LIBCPP_CONSTEXPR and directly write constexpr. This remarks concerns other usages below. | |
82 | It would be more readable to call it __false_value or alike, because __value seems too generic. | |
86 | Why Using Capital Letters? What about unifying a bit static assert messages to match other in libc++? Examples: static_assert(is_trivially_copyable<_Tp>::value, "std::atomic<Tp> requires that 'Tp' be a trivially copyable type"); static_assert(__is_cpp17_move_insertable<allocator_type>::value, "The specified type does not meet the requirements of Cpp17MoveInsertible"); static_assert(__bitop_unsigned_integer<_Tp>::value, "__rotl requires unsigned"); Maybe something like "<numbers> header requires a floating-point type" or mentioning the corresponding concept (floating_point)? | |
libcxx/utils/generate_feature_test_macro_components.py | ||
596 | It should be written as int(201907), otherwise it won't work on Python3. |
Thanks for review @curdeius
clang-formatted the diff.
and used http://mpmath.org/ for calculating all constants.
libcxx/test/std/language.support/support.limits/support.limits.general/version.version.pass.cpp | ||
---|---|---|
113 | It seems that you formatted a bit too much. I think that you should avoid formatting auto-generated files. |
libcxx/include/numbers | ||
---|---|---|
186 | Typo: gamma |
libcxx/include/numbers | ||
---|---|---|
189 | For the verification of the numbers, I'd put links to each sequence, e.g. https://oeis.org/A001620 for gamma. |
I've just realized that there is already D77505. You should probably think about helping to incorporate your changes into this other PR and abandon this one.
This needs to be mangled (__value).