This is an archive of the discontinued LLVM Phabricator instance.

[libc++][NFC] Cleanups in <charconv>.
ClosedPublic

Authored by Mordante on Mar 8 2022, 8:41 AM.

Details

Reviewers
Quuxplusone
philnik
Group Reviewers
Restricted Project
Commits
rG3925f98de4ac: [libc++][NFC] Cleanups in <charconv>.
Summary

Based on review comments in D97705 applied some code cleanups in
<charconv>. The header now uses a more recent libc++ style.

Diff Detail

Event Timeline

Mordante created this revision.Mar 8 2022, 8:41 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 8 2022, 8:41 AM
Mordante requested review of this revision.Mar 8 2022, 8:42 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 8 2022, 8:42 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
philnik accepted this revision as: philnik.Mar 8 2022, 8:48 AM
philnik added a subscriber: philnik.

LGTM % comment.

libcxx/include/charconv
109–111

I think this fails in C++03, but I don't know why it is available in C++03 at all.

Quuxplusone accepted this revision.Mar 8 2022, 9:45 AM

LGTM if CI is happy!

libcxx/include/charconv
109–111

+1; could this be moved under the following #if?

171

Pre-existing, tangential: I notice that here the SFINAE is testing explicit curly-brace-initialization from _Tp to uint32_t, but what's actually used on line 184 is implicit conversion (and ADL on _Tp). If _Tp is always a built-in integer type, this is probably all fine.

323–324

Consider breaking after int, to match lines 308-309.

This revision is now accepted and ready to land.Mar 8 2022, 9:45 AM
Mordante marked 4 inline comments as done.Mar 8 2022, 10:35 AM

Thanks for the reviews!

libcxx/include/charconv
109–111

Good point, we didn't support it in C++03, but missed the #if wasn't placed correctly.

171

The condition is_integral<_Tp>::value is true when this code is used. So it should be fine.

Mordante updated this revision to Diff 413869.Mar 8 2022, 10:36 AM
Mordante marked 2 inline comments as done.

Address review comments.

This revision was automatically updated to reflect the committed changes.