[libcxx-commits] [PATCH] D94924: [libc++] first steps of a private modulemap

Thu Jan 21 13:55:18 PST 2021

ldionne added a comment.

In D94924#2512639 <https://reviews.llvm.org/D94924#2512639>, @tschuett wrote:

> 1. You don't have to do anything.
> 2. If you use -fcxx-modules, it reduces the size of the surface of the std module. The std_private module is hidden from users. There is a " // FIXME: These should be private." in the module.modulemap. These headers should not be visible to users of the std module. The private module is exactly trying to do that.

Nice! Then yes, I think it's a good idea to do this. It all depends on what changes are needed of course, but I'd be favorable if the changes are reasonable. It definitely seems like an improvement to me.

================
Comment at: libcxx/include/__undef_macros_private:12
+#ifdef min
+#if !defined(_LIBCPP_DISABLE_MACRO_CONFLICT_WARNINGS)
+#if defined(_LIBCPP_WARNING)
----------------
tschuett wrote:
> ldionne wrote:
> > tschuett wrote:
> > > ldionne wrote:
> > > > Can you please indent nested preprocessor directives?
> > > It should be a plain copy of__undef_macros?!?
> > Wait, so why do we keep both `__undef_macros` and `__undef_macros_private`? Sorry if that's a basic question, I'm not very familiar with private module maps.
> Unfortunately, we have to keep both. Otherwise we get cyclic dependencies between the two modules (std and std_private).
Ugh! Are there other base headers that we'd have to duplicate this way, too? If there's no other solution but to duplicate, that would be a serious issue from my point of view. Duplicating this simple header is not a deal breaker, but anything more complex might be.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94924