This is an archive of the discontinued LLVM Phabricator instance.

[libc++][ranges] Mark completed Ranges papers and issues as done, bump version macro
ClosedPublic

Authored by var-const on Dec 12 2022, 5:27 PM.

Details

Summary

All C++20 Ranges papers and LWG issues are done, with the exception of
https://wg21.link/P2210R2 ("Superior String Splitting"), and marked as
such.

All of these were already implemented prior to this patch except bumping
the feature test macro __cpp_lib_ranges as required by
https://wg21.link/P2325R3 ("Views should not be required to be default
constructible"). Note that, even though P2325R3 was voted into C++23, it
was voted with a recommendation for vendors to retroactively apply the
change to C++20 (see https://github.com/cplusplus/papers/issues/1007).

Diff Detail

Event Timeline

var-const created this revision.Dec 12 2022, 5:27 PM
Herald added a project: Restricted Project. · View Herald TranscriptDec 12 2022, 5:27 PM
var-const requested review of this revision.Dec 12 2022, 5:27 PM
Herald added a project: Restricted Project. · View Herald TranscriptDec 12 2022, 5:27 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
var-const added inline comments.Dec 12 2022, 5:31 PM
libcxx/docs/ReleaseNotes.rst
44

This patch relies on https://reviews.llvm.org/D136268 being submitted.

52

Should we add any other papers that this patch marks as done (or that were marked as done recently) here? This one is probably worth calling out because it bumps the ranges version macro. I'm not sure if others are worth calling out since they focus on fixing defects rather than adding new features, what do you think?

libcxx/docs/Status/Cxx20Issues.csv
247

It looks like has-tuple-element's definition has been completely revamped in the Standard since that paper. I haven't done too much archeology on this, just verified that what we have is same as the current draft of the Standard.

libcxx/include/__ranges/join_view.h
231

This is the only thing I found that was missing.

libcxx/include/version
139

If we follow the suggestion from https://github.com/cplusplus/papers/issues/1007 to apply the change to C++20 retroactively, I presume it means we don't need different values of the macro for C++20 and C++23.

(Unfortunately, we missed the previous version bump of this macro to 201911L that should have happened when we marked P1716R3 as implemented)

philnik added inline comments.
libcxx/docs/ReleaseNotes.rst
52

I think it makes sense to list all papers. That allows people to quickly find out if a paper has been implemented that they are interested in, even if it's not a major change. I'm a lot more on the fence about LWG issues, since that would probably result in a lot of noise.

libcxx/include/version
139

Well, we'll need different values once we implement P2415R2 (which seems to be missing from the status page?), P2387R3 and P2494R2.

var-const added inline comments.Dec 12 2022, 6:40 PM
libcxx/include/version
139

Looking at https://github.com/cplusplus/papers/issues/1085, it appears that P2415R2 was also adopted as a DR against C++20. It is listed in docs/Status/Cxx20Papers.csv. But yeah, for the latter papers looks like we'll need different values.

tcanens added inline comments.
libcxx/include/__ranges/join_view.h
231
var-const retitled this revision from [libc++][ranges] Mark completed Ranges papers and issues as done, finish implementing P2325. to [libc++][ranges] Mark completed Ranges papers and issues as done, bump version macro.Dec 15 2022, 8:34 PM
var-const edited the summary of this revision. (Show Details)
var-const updated this revision to Diff 483423.Dec 15 2022, 8:35 PM
var-const edited the summary of this revision. (Show Details)

Address feedback.

var-const marked an inline comment as done.Dec 15 2022, 8:36 PM
var-const added inline comments.
libcxx/include/__ranges/join_view.h
231

Thanks a lot! I missed that issue, reverted the constraint now.

Mordante accepted this revision as: Mordante.Dec 20 2022, 11:17 AM
Mordante added a subscriber: Mordante.

Thanks for working on this! In general LGTM, but I didn't verify all papers. I leave the final approval to @philnik.

libcxx/docs/Status/Cxx20Issues.csv
247

Should the then mark it as Complete instead. Nothing to do implies nothing was ever done.

libcxx/include/__ranges/join_view.h
231

FYI D140407 adds that LWG issue to the status table.

libcxx/include/version
139

I applied the same logic to the DR papers for format, so I agree that's what we should do.

var-const marked 3 inline comments as done.Jan 17 2023, 10:54 PM
var-const added a subscriber: ldionne.
var-const added inline comments.
libcxx/docs/ReleaseNotes.rst
52

Personally, I would prefer to only list major papers (I don't like having to sift through a bunch of minor papers that are sometimes just outgrowth of LWG issues to find significant new features, if any). I don't feel too strongly about it, though.

@ldionne Louis, do you think it makes more sense to list all implemented papers (even the ones with minor fixes) or only the major ones? I think I'll merge this patch as is but happy to follow up tomorrow with a fuller list if that's what we ultimately decide.

libcxx/include/version
139

Thanks for this context!

var-const marked an inline comment as done.

Address feedback

This revision was not accepted when it landed; it landed in state Needs Review.Jan 17 2023, 10:55 PM
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Mordante added inline comments.Jan 18 2023, 8:23 AM
libcxx/docs/ReleaseNotes.rst
52

IMO it depends a bit on how user visible the change is. A small paper that has a lot of user visible impact I would like to see. I think we can even omit large papers, when they have little impact. There are some papers changing mandates clauses in the library, probably not something users are interested in. This one for example https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2020/p2045r1.html, but there are larger papers.

ldionne added inline comments.Jan 18 2023, 12:28 PM
libcxx/docs/ReleaseNotes.rst
52

I agree with Mark. Release notes are for things that we want to call out, they are not a laundry list of everything we've done. People have the status pages for that.