This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Improve make_string test support.
ClosedPublic

Authored by Mordante on May 13 2021, 9:27 AM.

Details

Summary

Adds MAKE_CSTRING and makes the operators of MultiStringType constexpr.

The code is copied from D96664 so it can be used in D80895.

Diff Detail

Event Timeline

Mordante requested review of this revision.May 13 2021, 9:27 AM
Mordante created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptMay 13 2021, 9:27 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
cjdb accepted this revision.May 13 2021, 9:35 AM
Quuxplusone accepted this revision.May 13 2021, 9:51 AM
Quuxplusone added a subscriber: Quuxplusone.

Sure; although here are some non-blockers:

  • the repeated comment is maybe overkill
  • the existing code's use of implicit conversion operators (instead of say an .as_chars<CharT>() getter) is just horrible and I'd love to see a PR to fix it throughout the test suite
  • I wonder if you're going to need MAKE_STRING_VIEW next
This revision is now accepted and ready to land.May 13 2021, 9:51 AM
curdeius accepted this revision.May 13 2021, 10:12 AM
curdeius added a subscriber: curdeius.

LGTM. I second what Arthur said.

Thanks for the reviews. I'll update the comment before landing.
I did quite a bit more on std::format than what's up for review. I haven't had the need for MAKE_STRING_VIEW so I don't foresee I'll ever need it :-)
I think you've a good point about the implicit conversion. I'll add it to my todo list.

This revision was automatically updated to reflect the committed changes.