[libcxx-commits] [PATCH] D153211: [libc++][Modules] Make module exports consistent with header includes

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added inline comments.


================
Comment at: libcxx/include/__memory/concepts.h:22
 #include <__type_traits/remove_cvref.h>
+#include <__type_traits/remove_reference.h>
 
----------------
Why is this one needed?


================
Comment at: libcxx/include/cwchar:113
 #include <cwctype>
+#include <stdio.h>
 
----------------
iana wrote:
> Mordante wrote:
> > Is this really needed, it looks weird; C headers should be independent.
> I think it was transitively included, but cwchar explicitly exports the module, so it needs to include the header.
Instead, what happens if we remove `export depr.stdio_h` from `module cwchar`? This seems like a better approach to me?


================
Comment at: libcxx/include/string_view:230
 #include <__type_traits/type_identity.h>
+#include <initializer_list>
 #include <iosfwd>
----------------
iana wrote:
> Mordante wrote:
> > The header does not use `initializer_list` why is it needed?
> string_view's module explicitly exports it, and this one I'm pretty sure wasn't being transitively included. So this export would definitely break without the include being added.
I think we want to remove the include and also remove the `export initializer_list` from `module string_view`. Some headers are specified to include `<initializer_list>`, but `<string_view>` isn't. I suspect the export was added incorrectly.


================
Comment at: libcxx/include/tgmath.h:27-28
 #ifdef __cplusplus
+#  include <ccomplex>
+#  include <cmath>
 #  include <ctgmath>
----------------
What happens if we remove the exports instead? Our "C compatibility" headers are normally just `#include_next`ing the C library header, and we should strive to keep them as close as possible to that.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D153211/new/

https://reviews.llvm.org/D153211