This is an archive of the discontinued LLVM Phabricator instance.

Differential D116691

[libc++] Add Status page for P2321R2 (Zip)
ClosedPublic

Authored by philnik on Jan 5 2022, 1:09 PM.

Details

Reviewers
• Quuxplusone
ldionne
Mordante
Group Reviewers
Restricted Project
Commits
rGd5b5fc06b85a: [libc++] Add Status page for P2321R2 (Zip)
Summary

Add a status page for P2321R2

Diff Detail

Event Timeline

philnik created this revision.Jan 5 2022, 1:09 PM
Herald added a subscriber: arphaman. · View Herald TranscriptJan 5 2022, 1:09 PM
philnik requested review of this revision.Jan 5 2022, 1:09 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 5 2022, 1:09 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
Herald added a subscriber: libcxx-commits. · View Herald Transcript
jloser added a subscriber: jloser.Jan 5 2022, 1:50 PM
jloser added inline comments.
libcxx/docs/Status/Zip.rst
18

s/imperitave/imperative

Harbormaster completed remote builds in B141756: Diff 397687.Jan 5 2022, 2:26 PM
• Quuxplusone added a comment.Jan 5 2022, 3:52 PM

LGTM % comments... although this would be another place we could try using GitHub issues instead of .csv files, if we wanted to. (I'm neutral.)

libcxx/docs/Status/Zip.rst
18–20

I'd just replace this whole sentence with This avoids duplicating effort. libc++ is hardly known for "effective implementation," ;) and I don't actually see how this has any effect on "implementors [getting] blocked by each other." In a sense, having this list (a synchronization mechanism) is what makes it possible for one person to get blocked by another; without it, you'd just duplicate the other person's work (which could lead to starvation of the slower worker, but never deadlock).

24
philnik updated this revision to Diff 397871.Jan 6 2022, 6:39 AM
philnik marked 3 inline comments as done.
  • Addressed comments
Harbormaster completed remote builds in B141893: Diff 397871.Jan 6 2022, 7:58 AM
Mordante added a comment.Jan 6 2022, 9:15 AM

Thanks for working on this!

libcxx/docs/Status/ZipProjects.csv
7

This line looks odd.

10

I miss [ranges.syn]. I like to see it since it's the only place with the enable_borrowed_range.

12

Should these two lines be one line? (I see similar issues on lines 14-15, 18-19, 22-23.)

philnik mentioned this in D115977: [libc++] Implement P1425R4 (Iterator pair constructors for std::stack and std::queue).Jan 6 2022, 9:40 AM
philnik updated this revision to Diff 397988.Jan 6 2022, 2:14 PM
philnik marked an inline comment as done.
  • Added enable_borrowed_range zip_view and adjacent_view task
libcxx/docs/Status/ZipProjects.csv
7

All these weird line breaks are forced line breaks in the generated table. I'm not exactly fluent in Sphinx, but I don't think there is a better way. At least it's done the same way in SpaceshipProjects.csv.

Harbormaster completed remote builds in B141967: Diff 397988.Jan 6 2022, 3:26 PM
Mordante added a comment.Jan 10 2022, 8:39 AM

The CI for the documentation has been enabled again, can you rebase this commit to see whether the CI is happy?

libcxx/docs/Status/ZipProjects.csv
7

I see I hadn't noticed that in spaceship before.

philnik updated this revision to Diff 398668.Jan 10 2022, 9:13 AM

Rebased

Harbormaster completed remote builds in B142461: Diff 398668.Jan 10 2022, 10:36 AM
ldionne accepted this revision as: ldionne.Jan 11 2022, 9:07 AM
Mordante accepted this revision as: Mordante.Jan 11 2022, 9:12 AM

LGTM, when you land this please make 2 commits

  • Zip status
  • Bug tracker updates
Mordante accepted this revision.Jan 11 2022, 9:15 AM

Nevermind. It seems when you select https://reviews.llvm.org/D116691/new/ you sometimes get unrelated changes :-(
I also noticed I'm the second reviewer.

This revision is now accepted and ready to land.Jan 11 2022, 9:15 AM
Closed by commit rGd5b5fc06b85a: [libc++] Add Status page for P2321R2 (Zip) (authored by philnik). · Explain WhyJan 11 2022, 2:15 PM
This revision was automatically updated to reflect the committed changes.
philnik added a commit: rGd5b5fc06b85a: [libc++] Add Status page for P2321R2 (Zip).

Revision Contents

PathSize
libcxx/
docs/
Status/
29 lines
27 lines
1 line

Diff 398668

libcxx/docs/Status/Zip.rst

  • This file was added.
.. zip-status:
===========================
libc++ Zip Status (P2321R2)
===========================
.. include:: ../Helpers/Styles.rst
.. contents::
:local:
Overview
========
This document contains the status of the C++2b zip implementation in libc++.
It is used to track both the status of the sub-projects of the effort and who
is assigned to these sub-projects. This avoids duplicating effort.
jloserUnsubmitted
Done
ReplyInline Actions

s/imperitave/imperative

jloser: `s/imperitave/imperative`
If you are interested in contributing to this effort, please send a message
to the #libcxx channel in the LLVM discord. Please *do not* start working
QuuxplusoneUnsubmitted
Done
ReplyInline Actions

I'd just replace this whole sentence with This avoids duplicating effort. libc++ is hardly known for "effective implementation," ;) and I don't actually see how this has any effect on "implementors [getting] blocked by each other." In a sense, having this list (a synchronization mechanism) is what makes it possible for one person to get blocked by another; without it, you'd just duplicate the other person's work (which could lead to starvation of the slower worker, but never deadlock).

Quuxplusone: I'd just replace this whole sentence with `This avoids duplicating effort.` libc++ is hardly…
on any items below that has already been assigned to someone else.
Sub-projects in the Implementation Effort
=========================================
QuuxplusoneUnsubmitted
Done
ReplyInline Actions
to the #libcxx channel in the LLVM discord. Please *do not* start working
- on any of the assigned items below.
+ on any item below that has already been assigned to someone else.
Sub-projects in the Implementation Effort
Quuxplusone:
.. csv-table::
:file: ZipProjects.csv
:header-rows: 1
:widths: auto

libcxx/docs/Status/ZipProjects.csv

  • This file was added.
Section,Description,Dependencies,Assignee,Complete
| `[tuple.syn] <https://wg21.link/tuple.syn>`_, "`[tuple] basic_common_reference, common_type <https://reviews.llvm.org/D116538>`_", None, Nikolas Klauser, |In Progress|
| `[tuple.tuple] <https://wg21.link/tuple.tuple>`_, "`[tuple] constructor, assignment and swap overloads <https://reviews.llvm.org/D116621>`_", None, Nikolas Klauser, |In Progress|
| `[utility.syn] <https://wg21.link/utility.syn>`_, "[pair] basic_common_reference, common_type", None, Nikolas Klauser, |Not Started|
| `[pairs.pair] <https://wg21.link/pairs.pair>`_, "[pair] constructor, assignment and swap overloads", None, Nikolas Klauser, |Not Started|
"| `[memory.syn] <https://wg21.link/memory.syn>`_
| `[allocator.uses.construction] <https://wg21.link/allocator.uses.construction>`_", "[pair] uses_allocator_construction_args overloads", None, Unassigned, |Not Started|
MordanteUnsubmitted
Not Done
ReplyInline Actions

This line looks odd.

Mordante: This line looks odd.
philnikAuthorUnsubmitted
Done
ReplyInline Actions

All these weird line breaks are forced line breaks in the generated table. I'm not exactly fluent in Sphinx, but I don't think there is a better way. At least it's done the same way in SpaceshipProjects.csv.

philnik: All these weird line breaks are forced line breaks in the generated table. I'm not exactly…
MordanteUnsubmitted
Not Done
ReplyInline Actions

I see I hadn't noticed that in spaceship before.

Mordante: I see I hadn't noticed that in spaceship before.
| `[vector.bool] <https://wg21.link/vector.bool>`_, "[vector<bool>::reference] add const operator= overload", None, Nikolas Klauser, |Not Started|
| `[iterator.concept.winc] <https://wg21.link/iterator.concept.winc>`_, "Update weakly_comparable", None, Unassigned, |Not Started|
| `[range.zip] <https://wg21.link/ranges.syn>`_, "zip_view", "| `zip_view::iterator`
MordanteUnsubmitted
Done
ReplyInline Actions

I miss [ranges.syn]. I like to see it since it's the only place with the enable_borrowed_range.

Mordante: I miss [ranges.syn]. I like to see it since it's the only place with the…
| `zip_view::sentinel`", Unassigned, |Not Started|
| `[range.zip.iterator] <https://wg21.link/range.zip.transform>`_, "zip_view::iterator", None, Unassigned, |Not Started|
MordanteUnsubmitted
Not Done
ReplyInline Actions

Should these two lines be one line? (I see similar issues on lines 14-15, 18-19, 22-23.)

Mordante: Should these two lines be one line? (I see similar issues on lines 14-15, 18-19, 22-23.)
| `[range.zip.sentinel] <https://wg21.link/range.zip.sentinel>`_, "zip_view::sentinel", None, Unassigned, |Not Started|
| `[range.zip.transform.view] <https://wg21.link/range.zip.transform.view>`_, "zip_transform_view", "| `zip_transform_view::iterator`
| `zip_transform_view::sentinel`", Unassigned, |Not Started|
| `[range.zip.transform.iterator] <https://wg21.link/range.zip.transform.iterator>`_, "zip_transform_view::iterator", None, Unassigned, |Not Started|
| `[range.zip.transform.sentinel] <https://wg21.link/range.zip.transform.sentinel>`_, "zip_transform_view::sentinel", None, Unassigned, |Not Started|
| `[range.adjacent.view] <https://wg21.link/range.adjacent.view>`_, "adjacent_view", "| `adjacent_view::iterator`
| `adjacent_view::sentinel`", Unassigned, |Not Started|
| `[range.adjacent.iterator] <https://wg21.link/range.adjacent.iterator>`_, "adjacent_view::iterator", None, Unassigned, |Not Started|
| `[range.adjacent.sentinel] <https://wg21.link/range.adjacent.sentinel>`_, "adjacent_view::sentinel", None, Unassigned, |Not Started|
| `[range.adjacent.transform.view] <https://wg21.link/range.adjacent.transform.view>`_, "adjacent_transform_view", "| `adjacent_transform_view::iterator`,
| `adjacent_transform_view::sentinel`", Unassigned, |Not Started|
| `[range.adjacent.transform.iterator] <https://wg21.link/range.adjacent.transform.iterator>`_, "adjacent_transform_view::iterator", None, Unassigned, |Not Started|
| `[range.adjacent.transform.sentinel] <https://wg21.link/range.adjacent.transform.sentinel>`_, "adjacent_transform_view::sentinel", None, Unassigned, |Not Started|
| `[ranges.syn] <https://wg21.link/ranges.syn>`_, "enable_borrowed_range zip_view and adjacent_view", "| `zip_view`
| `adjacent_view`", Unassigned, |Not Started|

libcxx/docs/index.rst

Show All 40 Lines.. toctree::
Contributing Contributing
Status/Cxx14 Status/Cxx14
Status/Cxx17 Status/Cxx17
Status/Cxx20 Status/Cxx20
Status/Cxx2b Status/Cxx2b
Status/Format Status/Format
Status/Ranges Status/Ranges
Status/Spaceship Status/Spaceship
Status/Zip
.. toctree:: .. toctree::
:hidden: :hidden:
AddingNewCIJobs AddingNewCIJobs
FeatureTestMacroTable FeatureTestMacroTable
▲ Show 20 Lines • Show All 167 Lines • Show Last 20 Lines