This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Don't pass the allocator in substr()
ClosedPublic

Authored by philnik on Nov 15 2022, 3:19 PM.

Details

Reviewers
ldionne
Mordante
Group Reviewers
Restricted Project
Commits
rGad79455fad9f: [libc++] Don't pass the allocator in substr()
Summary

This bug was dicoved when implementing P2438R2.

Fixes #57190

Diff Detail

Event Timeline

philnik created this revision.Nov 15 2022, 3:19 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 15 2022, 3:19 PM
philnik requested review of this revision.Nov 15 2022, 3:19 PM
Herald added a project: Restricted Project. · View Herald TranscriptNov 15 2022, 3:19 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
ldionne accepted this revision.Nov 21 2022, 8:56 AM

LGTM w/ comment. Also please rebase and get a green CI run.

libcxx/test/std/strings/basic.string/string.cons/substr.allocator.pass.cpp
1

I think this should be added to libcxx/test/std/strings/basic.string/string.ops/string_substr/substr.pass.cpp instead.

This revision is now accepted and ready to land.Nov 21 2022, 8:56 AM
Mordante accepted this revision.Nov 21 2022, 10:52 AM

LGTM, after addressing all review comments.

libcxx/test/std/strings/basic.string/string.cons/substr.allocator.pass.cpp
21

This comment sounds like this test tests libc++ specific behaviour. Can you add some guards to the libc++ specific tests so it will pass of other Standard libraries too?

philnik updated this revision to Diff 478541.Nov 29 2022, 5:33 AM
philnik marked 2 inline comments as done.

Address comments

libcxx/test/std/strings/basic.string/string.cons/substr.allocator.pass.cpp
21

The allocator isn't actually forwarded, so we always copy it. Removed the comment.

This revision was landed with ongoing or failed builds.Nov 29 2022, 10:40 AM
This revision was automatically updated to reflect the committed changes.