This is an implementation detail for move_only_function
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
libcxx/include/__math_h/log2i.h | ||
---|---|---|
10 | Why is this not just under __math? | |
24 | We might as well add a comment explaining what the function does (compute the base-2 log of an integral type)? | |
28 | Here and elsewhere? | |
libcxx/include/__utility/pointer_int_pair.h | ||
63–65 | I really feel like the parameters should be the other way around here, because this is really a pair, and we always specify both types in a pair. I suggest we do this: enum class __bitfield_width : size_t { }; template <class _Pointer, class _IntType, __bitfield_width __int_bit_count> struct __pointer_int_pair; This way, we can do this, which is more obvious than what we have right now IMO: __pointer_int_pair<Overaligned*, size_t, __bitfield_width(2)> In particular, I don't see why we have a default size_t type for the integral type. I don't think it makes sense, since this tries to be a general pointer-and-integral-type pair. | |
66 | I think this can be defined as a typedef inside the class? | |
80–85 | _LIBCPP_HIDE_FROM_ABI | |
87 | Please add a test that uses the implicit syntax here to lock in this behavior. I think it's useful to have this be non-explicit so you can do something like return {ptr, int} when you know that the return type is a __pointer_int_pair, but I think it should be locked with a test. | |
103–104 | I would hold off creating this until you need it (which may be in the very next patch, but then we can see it in context and decide whether it pulls its weight). | |
110–111 | I would just inline it like this. I missed the __pointer prefix when I first read it and I thought this was a bug -- I think it's a bit confusing and doesn't save a lot of typing. | |
116–118 | Those could be constexpr, not that it wins us very much. | |
libcxx/test/libcxx/utilities/pointer_int_pair/constructor_assert.pass.cpp | ||
2 | Let's name this assert.constructor.pass.cpp -- I've been trying to do this consistently. | |
libcxx/test/libcxx/utilities/pointer_int_pair/pointer_int_pair.pass.cpp | ||
14–17 | You could use ADDITIONAL_COMPILE_FLAGS: -Wno-private-header instead. | |
21 | Please test that you can constinit a pointer-int-pair. | |
23 | Please test with an overaligned type and with more than 2 bits for the integer. | |
25 | Please test with a non-std::size_t type, like bool and a few others. |
Mainly curious, can you give an indication how much space this would save?
libcxx/include/__math_h/log2i.h | ||
---|---|---|
10 | Actually I think this file should be removed. Instead we should make __bit_log2 in <bit> available in all language versions. |
This is heavily inspired from it, but we're still tweaking the API as we think makes the most sense. Since this is a private type, we can always end up changing the API if we find the we like something closer to the original one better.
Why is this not just under __math?