This is an archive of the discontinued LLVM Phabricator instance.

[libc++][charconv] Adds operator bool.
ClosedPublic

Authored by Mordante on Jun 17 2023, 6:32 AM.

Details

Reviewers
ldionne
Group Reviewers
Restricted Project
Commits
rG92ac3600630c: [libc++][charconv] Adds operator bool.
Summary

Implements

  • P2497R0 Testing for success or failure of <charconv> functions

Depends on D153192

Diff Detail

Event Timeline

Mordante created this revision.Jun 17 2023, 6:32 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 17 2023, 6:32 AM
Herald added a subscriber: arichardson. · View Herald Transcript
Mordante requested review of this revision.Jun 17 2023, 6:32 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 17 2023, 6:32 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
Mordante updated this revision to Diff 532397.Jun 17 2023, 6:36 AM

Rebased on dependency.

Mordante updated this revision to Diff 532403.Jun 17 2023, 9:10 AM

CI fixes.

philnik retitled this revision from [libc++][charconf] Adds operator bool. to [libc++][charconv] Adds operator bool..Jul 7 2023, 4:43 PM
Mordante updated this revision to Diff 538353.Jul 8 2023, 4:34 AM

Rebased and update status table.

Mordante updated this revision to Diff 541692.Jul 18 2023, 1:09 PM

Rebased to test CI.

Mordante updated this revision to Diff 541704.Jul 18 2023, 1:22 PM

Rebased since main was broken.

Mordante updated this revision to Diff 549587.Aug 12 2023, 3:27 AM

Rebased and target LLVM-18 as shipping vehicle.

ldionne accepted this revision.Aug 22 2023, 10:17 AM
ldionne added a subscriber: ldionne.

LGTM w/ comments and CI!

libcxx/test/std/utilities/charconv/charconv.syn/from_chars_result.operator_bool.verify.cpp
19 ↗(On Diff #549587)

I would suggest instead doing

static_assert(!std::is_convertible<std::from_chars_result, bool>::value);
static_assert( std::is_constructible<bool, std::from_chars_result>::value);

This way we are also testing that this is SFINAE-friendly, which tests explicit more tightly than if we check for the error. We also decouple from the error message and we're able to merge this test into the .pass.cpp test you added.

This revision is now accepted and ready to land.Aug 22 2023, 10:17 AM
Mordante marked an inline comment as done.Aug 23 2023, 8:35 AM
Mordante updated this revision to Diff 552731.Aug 23 2023, 8:36 AM

Rebased and address review comments.

This revision was automatically updated to reflect the committed changes.