This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Add <ext/hash_{set,map}> to the modulemap
AbandonedPublic

Authored by Quuxplusone on Mar 1 2022, 11:30 AM.

Details

Reviewers
ldionne
philnik
jyknight
Group Reviewers
Restricted Project
Summary

Inspired by D118616. This should permit <ext/__hash> to
include <__string> without triggering a "private submodule"
diagnostic (which is currently suppressed because system header,
but might soon become visible).

(I'm not claiming that this gets us exactly where we want to be. From my discussion with @jyknight on Discord it actually sounds like the existing modulemap is pretty much BS from the user's point of view, and that in order to implement http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2022/p2465r2.pdf we might have to burn it to the ground and start over. But I do hypothesize that making this change here will permit @philnik to remove some of the diffs in D118616 that are concerned with -Wprivate-header.)

Diff Detail

Event Timeline

Quuxplusone requested review of this revision.Mar 1 2022, 11:30 AM
Quuxplusone created this revision.
Herald added 1 blocking reviewer(s): Restricted Project. · View Herald TranscriptMar 1 2022, 11:30 AM
ldionne accepted this revision.Mar 1 2022, 11:36 AM

I agree with the statement that our current modulemap is probably not good for P2465R2. However, missing ext/hash_map and ext/hash_set is just an inconsistency, so this is still an improvement over the status quo. LGTM if CI passes.

This revision is now accepted and ready to land.Mar 1 2022, 11:36 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 1 2022, 12:59 PM
philnik accepted this revision.Mar 2 2022, 8:01 AM
Quuxplusone abandoned this revision.Mar 2 2022, 9:36 AM

Local experimentation showed that this didn't actually help D118616 at all, so this idea has been superseded by:

  • Add UNSUPPORTED: modules-build to the ext/hash tests as part of D118616, and/or
  • Rip out ext/hash entirely in D120831.