This addresses a request during the review of D128929.
Details
- Reviewers
ldionne - Group Reviewers
Restricted Project - Commits
- rG77ccf63ef0c4: [libc++][doc] Extended integral type support
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
libcxx/docs/UsingLibcxx.rst | ||
---|---|---|
453–454 | Could you maybe add a TODO somewhere to update the function names to match the standard ones? |
Thanks for the review!
libcxx/docs/UsingLibcxx.rst | ||
---|---|---|
453–454 | I'm not sure whether we want to update these names. On one hand it's nice to use the C++20 name on the other hand I like to use the intrinsic's name. But I have no strong preference in either direction. I'm not going to add a TODO. If we want to use the C++20 names I'll do the renaming in a separate patch as a parent of this one and adjust this one to the new names. |
libcxx/docs/UsingLibcxx.rst | ||
---|---|---|
453–454 | Normally we mirror the names of the standard ones. This makes a lot of sense IMO, since people are much more likely to be familiar with the standard names than with intrinsics. Personally I also find the names countl_zero and countr_zero a lot more intuitive than ctz and clz. Unless you already know that ctz is count trailing zeros you would never guess that. |
libcxx/docs/UsingLibcxx.rst | ||
---|---|---|
453–454 | I don't object, but let's agree on the rename before doing the work. |
libcxx/docs/UsingLibcxx.rst | ||
---|---|---|
440 | My comment in https://reviews.llvm.org/D128929#inline-1241329 was asking to document the fact that __uint128_t can be used with *public* functions in <bits> (and also in a few other places like <random>, <charconv>, <type_traits>, etc). I don't think we should document internal-only functions and headers, since this is asking for trouble when users start depending on them. I'm sorry if my comment in the other review was not clear enough. | |
453–454 | I'm mostly neutral on this, but I guess it would be more consistent to name them after their standardized names. However, note that this is only beneficial for libc++ developers themselves, since we never expect users to use these names. |
LGTM, but you'll have to rebase on top of the changes I made in 07e984bc52014a8565033be10f7e80982e974b99.
My comment in https://reviews.llvm.org/D128929#inline-1241329 was asking to document the fact that __uint128_t can be used with *public* functions in <bits> (and also in a few other places like <random>, <charconv>, <type_traits>, etc). I don't think we should document internal-only functions and headers, since this is asking for trouble when users start depending on them.
I'm sorry if my comment in the other review was not clear enough.