[libcxx-commits] [PATCH] D144322: [libc++][Modules] Make top level modules for all C++ headers with OS/clang versions

[Duncan Exon Smith via libcxx-commits]

I don’t have an strong opinion about whether the cover headers are worth adding, but IMO without those it’s better to drop the do-nothing exports. I.e., add the headers or delete the no-longer meaningful modules. 

(I have a slight preference for deleting, since it seems like we could get away with it, but I’d rather someone more actively involved made the call.)

> On Feb 23, 2023, at 13:07, Ian Anderson via Phabricator <reviews at reviews.llvm.org> wrote:
> 
> iana added inline comments.
> 
> 
> ================
> Comment at: libcxx/include/module.modulemap.in:12
>     module ctype_h {
> -      header "ctype.h"
> -      export *
> +      export std_ctype_h
>     }
> ----------------
> iana wrote:
>> I'm almost positive all these export lines don't do anything besides inform the reader where the header has moved. This seems to be the style in Apple's module maps when headers move, but I'm not sure if there's any practical reason for doing it. Keeping the modules around would make it so that if anyone actually did `@import std.depr.ctype_h;` then that wouldn't result in an error. They'd probably still get ctype.h via something else in `std` including that header, but it wouldn't be guaranteed.
>> 
>> We could make underscored headers for all of these that just include the regular version to force `std` to transitively include all of the new top level modules. We could also make a single "dependencies" header that includes all of them, and add that to the `std` module. Or maybe the way it is is fine?
> I did some testing, and these `export` lines do absolutely nothing. If we want to make them actually export the new top level modules, we need cover headers like _ctype.h that includes ctype.h purely for module re-export purposes. Probably with a `__building_module` guard too. Or, just get rid of these submodules completely. I think it's only a problem if there are some clients out there that `import std.depr.ctype_h`, but that seems pretty unlikely to me.
> 
> 
> Repository:
>  rG LLVM Github Monorepo
> 
> CHANGES SINCE LAST ACTION
>  https://reviews.llvm.org/D144322/new/
> 
> https://reviews.llvm.org/D144322
>