[libc++] first steps of a private modulemap

[tschuett]

Summary

Note that __config is not a module anymore.

Note that there are a lot of cyclic dependencies hidden by the main module.

[tschuett]

add newline

[tschuett]

more pseudo-private modules

[tschuett]

finally managed to get __bits running

[ldionne]

Is this ready for review? If not, please ping me when it is. In the meantime, I'll "request changes" so it shows up correctly in the queue.

libcxx/include/__undef_macros_private

12 ↗

(On Diff #317423)

Can you please indent nested preprocessor directives?

libcxx/include/module.modulemap

4

Why is this not an issue anymore? We still reference this module below in module std, don't we?

[tschuett]

You can see it as proof of concept. If you don't like, feel free to reject.

libcxx/include/__undef_macros_private

12 ↗

(On Diff #317423)

It should be a plain copy of__undef_macros?!?

libcxx/include/module.modulemap

4

The problem is that there are two modules in module.modulemap. Private modules don't work with having two modules.

[tschuett]

libcxx/include/__undef_macros_private

2 ↗

(On Diff #317423)

undef_macros_private is an exact copy of undef_macros. I didn't even change the name.

[ldionne]

Generally speaking, I have no objection with doing this if it's an improvement. But I don't understand the benefits / tradeoffs here cause I don't have much experience with Clang modules (so far the libc++ Clang modules are kind of a side thing maintained by people who care about it, but we still use the non-modular build by default). Can you explain what introducing private modules is going to buy us, and what sort of changes are going to be required? Will this have any visible change for people building with -fcxx-modules or -fmodules today?

libcxx/include/__undef_macros_private

12 ↗

(On Diff #317423)

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.

[tschuett]

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.

libcxx/include/__undef_macros_private

12 ↗

(On Diff #317423)

Unfortunately, we have to keep both. Otherwise we get cyclic dependencies between the two modules (std and std_private).

[ldionne]

In 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.

libcxx/include/__undef_macros_private

12 ↗

(On Diff #317423)

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.

[curdeius]

libcxx/include/module.modulemap

526

Is this order change intended? It was alphabetically sorted before...

[tschuett]

• rebased
• sorted headers

[tschuett]

I would rather expect that we have to split some headers into smaller/modular ones.

[ldionne]

In D94924#2514826, @tschuett wrote:

I would rather expect that we have to split some headers into smaller/modular ones.

Yes, indeed. I would 100% support that. In fact, I've started doing that here and there when it makes sense. If you want to pursue such an effort, I would definitely back it up.

[tschuett]

I would rather see it as that I am forced to do that. It is the only way to eliminate cyclic dependencies.

[ldionne]

Is this still relevant? Our support for modules has evolved enormously in the past few months. If not, can we please abandon this review to clear up the review queue?

libcxx/include/CMakeLists.txt

set(files
   limits.h
   list
   locale
   locale.h
   map
   math.h
   memory
   module.modulemap
+  module.private.modulemap
   mutex
   new
   numbers
   numeric
   optional
   ostream
   queue
   random

libcxx/include/module.modulemap

-// define the module for __config outside of the top level 'std' module
-// since __config may be included from C headers which may create an
-// include cycle.
-module std_config [system] [extern_c] {
-    header "__config"
-}
-
 module std [system] {
   export std_config
   // FIXME: The standard does not require that each of these submodules
   // re-exports its imported modules. We should provide an alternative form of
   // export that issues a warning if a name from the submodule is used, and
   // use that to provide a 'strict mode' for libc++.
 
   // Deprecated C-compatibility headers. These can all be included from within
module std [system] {
   module __bits { header "__bits" export * }
   module __bit_reference { header "__bit_reference" export * }
   module __debug { header "__debug" export * }
   module __errc { header "__errc" export * }
   module __functional_base { header "__functional_base" export * }
   module __hash_table { header "__hash_table" export * }
   module __locale { header "__locale" export * }
   module __mutex_base { header "__mutex_base" export * }
   module __split_buffer { header "__split_buffer" export * }
-  module __sso_allocator { header "__sso_allocator" export * }
-  module __std_stream { header "__std_stream" export * }
   module __string { header "__string" export * }
   module __tree { header "__tree" export * }
   module __tuple { header "__tuple" export * }
   module __undef_macros { header "__undef_macros" export * }
   module __node_handle { header "__node_handle" export * }
 
   module experimental {
     requires cplusplus11

libcxx/include/module.private.modulemap

+module std_Private {
+
+  module __sso_allocator { header "__sso_allocator" export * }
+  module __std_stream { header "__std_stream" export * }
+
+  module __support { umbrella "support" export * }
+}
 No newline at end of file