This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Use bounded iterators in std::string_view when the debug mode is enabled
ClosedPublic

Authored by ldionne on Jan 30 2023, 8:35 AM.

Diff Detail

Event Timeline

ldionne created this revision.Jan 30 2023, 8:35 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 30 2023, 8:35 AM
Herald added a subscriber: arphaman. · View Herald Transcript
ldionne requested review of this revision.Jan 30 2023, 8:35 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 30 2023, 8:35 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript

Note that I thought I had fixed all of std::format's reliance on char*, but I have not so this will still fail for now.

ldionne updated this revision to Diff 493372.Jan 30 2023, 11:33 AM

Fix assumptions of std::string_view::iterator being char*

ldionne added a comment.EditedJan 30 2023, 2:06 PM

I think this looks good from the CI side. @Mordante I'll ship this if you are happy with the std::format changes.

Mordante accepted this revision.Jan 31 2023, 11:29 AM

Thanks a lot for working on this and fixing the format header.
LGTM modulo some nits.

libcxx/test/std/strings/string.view/string.view.iterators/debug.iterator-indexing.pass.cpp
7 ↗(On Diff #493372)

Why is this not a libc++ specific test?

18–20 ↗(On Diff #493372)

Unused.

27 ↗(On Diff #493372)

Should we also test with cfoo and crfoo?

This revision is now accepted and ready to land.Jan 31 2023, 11:29 AM
ldionne updated this revision to Diff 493705.Jan 31 2023, 12:22 PM
ldionne marked 3 inline comments as done.

Address comments -- thanks!

This revision was landed with ongoing or failed builds.Jan 31 2023, 3:24 PM
This revision was automatically updated to reflect the committed changes.