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

Wed Jun 28 14:10:23 PDT 2023

iana marked 9 inline comments as done.
iana added inline comments.

================
Comment at: libcxx/include/cwchar:113
 #include <cwctype>
+#include <stdio.h>

----------------
ldionne wrote:
> 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?
I'll try that with the rest of these. I've been hesitant to remove the exports because their lack causes linker errors post-module-split that are sensitive to exactly what gets included and which templates are used. The CI catches a lot of them, but probably not all. Maybe world building everything at Apple will shake out the rest of them, I was just going for a more conservative approach and keeping what was there working.

================
Comment at: libcxx/utils/libcxx/test/header_information.py:149-152
+                                                                     # Textual headers shouldn't be included directly
+                                                                     # (they're typically part of another header that
+                                                                     # was split out just for organization).
+                                                                     and str(p.relative_to(include)) not in private_textual_headers
----------------
ldionne wrote:
> Is this still going to be required after we make `__iterator/readable_traits.h` a standalone header?
It will be required in a later change, but not this one anymore.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153211