Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[unord.req] Move library name index entries to library name index. #1515

Closed
wants to merge 1 commit into from

Conversation

Eelis
Copy link
Contributor

@Eelis Eelis commented Mar 5, 2017

Library names such as max_bucket_count and rehash are currently absent from the index of library names, because their index entries use \indextext instead of \indexlibrary.

@AlisdairM
Copy link
Contributor

These should be using \indexlibraryname instead, but my grand indexing project stalled on clause 23 (now 26), and I need to check if I landed regex and locales as well.

@Eelis
Copy link
Contributor Author

Eelis commented Mar 7, 2017

Ah, sounds great! Then I'll wait until that work hits master, and amend this patch to use the new macros (because I see \indexlibraryname does not exist yet). Or feel free to close this ticket if these index entries will be covered by your patches anyway.

@AlisdairM
Copy link
Contributor

Doh! That's because I gave the wrong macro name, it should be \indexlibrarymember. I'm not sure that I will have a chance to complete the clause 23 (now 26) indexing before our DIS deadline, as naturally the editor will want to ship a stable document asap. Please don't hold this branch up waiting for mine to land - just use the appropriate macro, which will provide two index entries each time, listing as a member under the class, and the class under the member-name, so it can be looked up either way.

@Eelis
Copy link
Contributor Author

Eelis commented Mar 7, 2017

Ah, I see. The problem with using \indexlibrarymember is that it puts \idxcode around both arguments, while the index entries that this patch changes need it only on one (because "unordered associative containers" is not code).

@Eelis
Copy link
Contributor Author

Eelis commented Mar 17, 2017

Rebased.

@AlisdairM
Copy link
Contributor

I'm not opposed to the change as a strict improvement for C++17 DIS, but I don't think this is the right long-term answer. For example, we don't have any index entry under 'key_type' for the regular associative containers.

I still need to finish my indexing project for clause 26 (was 23), which has been stymied partly by so many member functions being defined in generic requirements tables. I will fold a follow-up to this change into that more considered indexing of the clause.

Given I will not have that pull request ready before the weekend, I suspect it will not land in 17, but may be deferred to 20.

@tkoeppe
Copy link
Contributor

tkoeppe commented Mar 18, 2017

@AlisdairM: I'm pretty sure we can apply index improvements before we ship the IS.

@zygoloid
Copy link
Member

I like that this moves index entries for library names to the library index. I don't like that it adds index entries for non-library names to the library index.

@zygoloid zygoloid added this to the C++20 milestone Jul 15, 2017
Eelis added a commit to Eelis/draft that referenced this pull request Jul 17, 2017
Eelis added a commit to Eelis/draft that referenced this pull request Jul 24, 2017
Eelis added a commit to Eelis/draft that referenced this pull request Jul 24, 2017
@tkoeppe
Copy link
Contributor

tkoeppe commented Aug 2, 2017

@zygoloid: Should we bite the bullet and spell out the names of the containers, so that no non-library names end up in the library index?

@Eelis
Copy link
Contributor Author

Eelis commented Aug 13, 2017

@tkoeppe If that is the goal, INVOKE should also be removed from the library index.

@zygoloid
Copy link
Member

@tkoeppe I'm not sure which non-library names you're referring to here.

@Eelis Yes, INVOKE should also be removed from the index of library names. It really should be an index of names declared by the library (that is, actual names visible in a C++ program). INVOKE should be in the main index. Likewise for "complex, literals".

@tkoeppe
Copy link
Contributor

tkoeppe commented Oct 16, 2017

@zygoloid: Something like "unordered associative container".

@tkoeppe
Copy link
Contributor

tkoeppe commented Nov 12, 2017

Editorial review: We don't see a need for this change at this point. As it stands, the library names are already covered via the synopses, and indexing the requirement table only through the regular index is sufficient.

Richard was entertaining the hypothetical idea that we could have an invented library-reserved name that is never declared, but used (e.g. a name required from the user), and that such a name should go into the library index. However, we don't actually have such a situation at the moment, and we can revisit this if we need to.

@tkoeppe tkoeppe closed this Nov 12, 2017
@Eelis Eelis deleted the unordindex branch July 3, 2020 03:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants