This is an archive of the discontinued LLVM Phabricator instance.

[libcxx][NFC] removes header synopses
AbandonedPublic

Authored by cjdb on Jun 28 2021, 10:00 PM.

Details

Reviewers
ldionne
EricWF
zoecarver
Mordante
Quuxplusone
jfb
Group Reviewers
Restricted Project
Summary

It probably made a lot of sense to include the headers' synopses in the
files themselves back when libc++ was first being implemented, but we're
a more connected world a decade on. It's very easy to look up synopses
using wg21.link, and we've also got cppreference, which is a
well-maintained wiki.

The synopses are essentially repeating a lot of the code, and they're a
maintenance burden that has plenty of evidence showing that they're not
always well-maintained. With our transition to more fine-grained
headers, it's also possible to consider these additional headers like a
synopsis summary.

As such, it's not really clear who the header synopses serve, except
maybe Big Storage (not really: we're only saving a little under 614KiB).

Diff Detail

Event Timeline

cjdb created this revision.Jun 28 2021, 10:00 PM
cjdb requested review of this revision.Jun 28 2021, 10:00 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 28 2021, 10:00 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
ldionne requested changes to this revision.Jun 29 2021, 7:35 AM

The purpose of those synopses is not to document what the Standard says, but instead to document what the state of our library implementation is. As such, they are often useful to figure out what has been implemented and what's missing from the library. They are especially useful when trying to figure out what parts of a partially implemented paper are missing (for the numerous papers where we only have "In Progress" in our tables). I also find that they help when reviewing patches, since they act as a summary of what the patch is doing (at least in most cases where we're implementing something new).

It is true they are a maintenance burden, but how much time do you *actually* spend updating those? It's really not a major source of wasted time in my experience. The main problem IMO is that we don't have a way to remind ourselves to update the synopses. If we were on GitHub, for example, we'd have a checklist reminder on pull requests that would contain a few things like "did you update the modulemap?" and "did you update the synopses?". The synopses would be more up-to-date and I don't think we'd have that conversation.

For all those reasons, I think we should keep synopses around, but I'm open to discussion.

This revision now requires changes to proceed.Jun 29 2021, 7:35 AM

I agree with what @ldionne said.
So I too would prefer to keep the synopsis to document our implementation status.

I only hope a CI job can do some of these checks, especially "did you update the modulemap?"

cjdb added a comment.Jun 29 2021, 9:11 AM

The purpose of those synopses is not to document what the Standard says, but instead to document what the state of our library implementation is. As such, they are often useful to figure out what has been implemented and what's missing from the library. They are especially useful when trying to figure out what parts of a partially implemented paper are missing (for the numerous papers where we only have "In Progress" in our tables). I also find that they help when reviewing patches, since they act as a summary of what the patch is doing (at least in most cases where we're implementing something new).

I'm not really seeing how they document the state of the library implementation. If one wants to know if something is implemented, we can just grep for it? If this info is genuinely valuable (and I'm seeing your point about "In Progress"), then I think it would be worth us moving the synopses out of the code and into status documents. I think that would be immensely more useful than having them in the same document, because they're more accessible (we can link "In Progress" statuses to specific webpages), and because it's not duplicating the interface in the code.

It is true they are a maintenance burden, but how much time do you *actually* spend updating those? It's really not a major source of wasted time in my experience. The main problem IMO is that we don't have a way to remind ourselves to update the synopses. If we were on GitHub, for example, we'd have a checklist reminder on pull requests that would contain a few things like "did you update the modulemap?" and "did you update the synopses?". The synopses would be more up-to-date and I don't think we'd have that conversation.

For all those reasons, I think we should keep synopses around, but I'm open to discussion.

FWIW, I am neutral on the major question; I don't find the synopses useful, but also don't mind them.

  • if we do ever remove the synopses, IMO we should replace them with nothing, not with // Synopsis available at http://wg21.link/algorithm.syn comments
  • <version> is autogenerated; if you want those changes to stick, you need to modify libcxx/utils/generate_feature_test_macro_components.py
  • anyway, it sounds like these points are moot for now :)

The purpose of those synopses is not to document what the Standard says, but instead to document what the state of our library implementation is. As such, they are often useful to figure out what has been implemented and what's missing from the library. They are especially useful when trying to figure out what parts of a partially implemented paper are missing (for the numerous papers where we only have "In Progress" in our tables). I also find that they help when reviewing patches, since they act as a summary of what the patch is doing (at least in most cases where we're implementing something new).

I'm not really seeing how they document the state of the library implementation. If one wants to know if something is implemented, we can just grep for it? If this info is genuinely valuable (and I'm seeing your point about "In Progress"), then I think it would be worth us moving the synopses out of the code and into status documents. I think that would be immensely more useful than having them in the same document, because they're more accessible (we can link "In Progress" statuses to specific webpages), and because it's not duplicating the interface in the code.

I'm not against moving to more detailed status pages instead of a synopsis. But then we should make sure that we add these pages for the projects in a partly done stage, instead of only removing this information.

cjdb added a comment.Jun 29 2021, 11:10 AM

The purpose of those synopses is not to document what the Standard says, but instead to document what the state of our library implementation is. As such, they are often useful to figure out what has been implemented and what's missing from the library. They are especially useful when trying to figure out what parts of a partially implemented paper are missing (for the numerous papers where we only have "In Progress" in our tables). I also find that they help when reviewing patches, since they act as a summary of what the patch is doing (at least in most cases where we're implementing something new).

I'm not really seeing how they document the state of the library implementation. If one wants to know if something is implemented, we can just grep for it? If this info is genuinely valuable (and I'm seeing your point about "In Progress"), then I think it would be worth us moving the synopses out of the code and into status documents. I think that would be immensely more useful than having them in the same document, because they're more accessible (we can link "In Progress" statuses to specific webpages), and because it's not duplicating the interface in the code.

I'm not against moving to more detailed status pages instead of a synopsis. But then we should make sure that we add these pages for the projects in a partly done stage, instead of only removing this information.

Right, by "status page", I mean "cut the synopses out of the headers and paste them into *.rst files verbatim" as a first step.

cjdb abandoned this revision.Jul 27 2021, 4:45 PM

FWIW, I think the usefulness of faithful and up-to-date synopses is more useful for the maintainer than for users and casual contributors. I find myself in need to "figure out what's the state of our implementation" fairly frequently, and being able to do that quickly by looking at the synopsis is useful. Since it's now mentioned in the Contributing.rst page, I think they should stay up-to-date and it's a small enough burden when making a patch.