This is an archive of the discontinued LLVM Phabricator instance.

[libc++][spaceship][NFC] Updates `SpaceshipProjects.csv` with full details from `P1614R2`
ClosedPublic

Authored by H-G-Hristov on Jun 6 2023, 11:54 AM.

Details

Summary

Adds the remaining sections from P1614R2
Some entries were reordered for easier tracking.
The items in the table match P1614R2's sections strictly.

Diff Detail

Event Timeline

H-G-Hristov created this revision.Jun 6 2023, 11:54 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 6 2023, 11:54 AM
Herald added a subscriber: yaxunl. · View Herald Transcript
H-G-Hristov retitled this revision from Updated `SpaceshipProjects.csv` to [libcxx][spaceship][NFC] Updates `SpaceshipProjects.csv` with full details from `P1614R2`.Jun 6 2023, 11:57 AM
H-G-Hristov edited the summary of this revision. (Show Details)
H-G-Hristov edited the summary of this revision. (Show Details)Jun 6 2023, 12:00 PM
H-G-Hristov retitled this revision from [libcxx][spaceship][NFC] Updates `SpaceshipProjects.csv` with full details from `P1614R2` to [libc++][spaceship][NFC] Updates `SpaceshipProjects.csv` with full details from `P1614R2`.

Updated table

Updates + Rebased

H-G-Hristov published this revision for review.Jun 7 2023, 6:56 AM
H-G-Hristov added a reviewer: Mordante.
Herald added a project: Restricted Project. · View Herald TranscriptJun 7 2023, 6:56 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
H-G-Hristov edited the summary of this revision. (Show Details)Jun 7 2023, 7:46 AM
H-G-Hristov edited the summary of this revision. (Show Details)

Thanks for working on this! I want to look at it in more detail later this week. I want to make sure we have all missing parts of the paper listed.

libcxx/docs/Status/SpaceshipPapers.csv
3 ↗(On Diff #529265)
libcxx/docs/Status/SpaceshipProjects.csv
178–182

Can you put these in progress and assign them to me? I have a(n ancient) patch for this.

190

This seems like an unwanted left-over.

H-G-Hristov marked 3 inline comments as done.

Addressed review comments

Thanks for working on this! I want to look at it in more detail later this week. I want to make sure we have all missing parts of the paper listed.

To clarify my motivation for the current format: As I am not that familiar with the structure of the format to make the section of the paper easier to track I decided to add all sections of the standard, which were changed in the paper to the paper status list. Sometimes some items (e.g. ".syn") contain all requirements and sometimes the implementation details are given in a separate section. I added all of them. I have not updated these extra sections yet.
My plan is for me (or if somebody else chips in) is to start from the top of the list check the implementation mark everything which is done and implement the missing parts in subsequent series of patches.

Please note some sections were removed or renamed by subsequent papers. Some sections are probably just an improvement to the wording in standard.

libcxx/docs/Status/SpaceshipPapers.csv
3 ↗(On Diff #529265)

I noticed this but it seems to work as expected and redirect to the most current version so I left it unchanged. Do you still want me to update it?

PS. DONE

libcxx/docs/Status/SpaceshipProjects.csv
190

Ah, sorry! I meant to remove it! Thanks for noticing it.

Marked items as "In Progress" and "Complete"

@Mordante Do we need to update the "Release Notes" also?

Marked more items as "In Progress"

Marked slice as "In Progress"

H-G-Hristov edited the summary of this revision. (Show Details)Jun 10 2023, 4:19 AM

@Mordante Do we need to update the "Release Notes" also?

I think we can wait. I really hope we can do all of P1614 before the next release and just mark it as done.
The next release will be branched at the end of July.

Thanks a lot for updating the status!
In general it looks great, just a few minor points. Can you ping me on Discord after updating the patch. Then I will quickly do a final check and can we land this.

libcxx/docs/Status/SpaceshipPapers.csv
3 ↗(On Diff #529265)

FYI typically this works, but sometimes a new revision of the paper is posted after it has been accepted by the C++ Committee. In these cases we still implement the accepted revision and not the new one.

Note this is very rare.

libcxx/docs/Status/SpaceshipProjects.csv
91

Please check whether the suggested markup is correct.

118
159

Can you add a new group here for removals of operator!=. The missing entries are the weekdays:

  • weekday
  • weekday_indexed
  • weekday_last
  • month_weekday
  • month_weekday_last
  • year_month_weekday
  • year_month_weekday_last

It think it makes sense to do this in one patch. If you're interested the calendar has been added in C++20, to the operators can be removed.

Addressed comments

H-G-Hristov marked 3 inline comments as done.

Removed wrong entry; month_day_last

H-G-Hristov marked an inline comment as done.Jun 10 2023, 7:35 AM

@Mordante Thank you for the review. I addressed the comments.

Fixed: duplicated entries in table

Mordante accepted this revision.Jun 10 2023, 8:17 AM

Thanks LGTM!

This revision is now accepted and ready to land.Jun 10 2023, 8:17 AM