This is an archive of the discontinued LLVM Phabricator instance.

[libc++] basic_string::resize_and_overwrite: Adopt LWG3645 (Not voted in yet)
ClosedPublic

Authored by philnik on Jan 7 2022, 8:40 AM.

Details

Summary

Adopt LWG3645, which fixes the value categories of basic_string::resize_and_overwrite
https://timsong-cpp.github.io/lwg-issues/3645

Diff Detail

Event Timeline

philnik requested review of this revision.Jan 7 2022, 8:40 AM
philnik created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptJan 7 2022, 8:40 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript

FWIW, I think this is a good idea (and might motivate LWG to DTRT), but I'm biased. Let's see what @ldionne thinks.
Also, I don't know if @ldionne would want us to mention LWG3645 in some csv file, or say nothing until it's adopted (and of course that question becomes moot if we just defer landing this at all).

libcxx/test/std/strings/basic.string/string.capacity/resize_and_overwrite.pass.cpp
80

You could actually add line 79 to the test today, right? It should work both before and after this patch.

Also, I don't know if @ldionne would want us to mention LWG3645 in some csv file, or say nothing until it's adopted (and of course that question becomes moot if we just defer landing this at all).

One of the problems with straying from our custom of never shipping something until it's in the Standard is that we don't have a process for doing it. For instance, if we make this change, do we just avoid marking the issue as implemented in the status files because there's no entry for it, and then we mark it as having been implemented since <some LLVM version> once the issue resolution lands in the Standard? That seems somewhat hand-wavy, and if we all died tomorrow (hopefully not), our successors wouldn't have a way to easily tell what happened without digging out this discussion.

This seems like a small enough issue that I wouldn't oppose shipping its resolution even before it's been voted officially, but if we did that, I would like to add an entry in the csv file to track issues we implement that haven't been voted yet.

[And once we have a nice place to put those, I would have a harder time fighting back people who want to ship stuff that's not in the Standard -- but I'm ready for it :-)]

libcxx/include/string
982

WDYT about using _LIBCPP_AUTO_CAST here? It would be closest to the standard.

This seems like a small enough issue that I wouldn't oppose shipping its resolution even before it's been voted officially, but if we did that, I would like to add an entry in the csv file to track issues we implement that haven't been voted yet.

To clarify, perhaps I'd suggest adding a new tag to https://libcxx.llvm.org/Status/Cxx2b.html saying something like not-voted-yet, or a new column. Or maybe another table after the LWG issues table. I don't mind the details, but I'd like *some* way of tracking what we're doing here.

libcxx/include/string
982

You mean

__erase_to_end(_VSTD::move(__op)(data(), _LIBCPP_AUTO_CAST(__n)));

? I think that's heavier-weight (in terms of #includes and template instantiations) than +__n, but I don't object.
(We could do _LIBCPP_AUTO_CAST(data()) too, but I think we both agree that'd be silly? :)

philnik updated this revision to Diff 401601.Jan 20 2022, 4:56 AM
philnik marked 2 inline comments as done.
  • Address comments
  • Add LWG issue entry
ldionne accepted this revision.Jan 20 2022, 8:16 AM
This revision is now accepted and ready to land.Jan 20 2022, 8:16 AM