[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

13

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

13

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

3

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

13

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

13

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

13

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

525

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
   __split_buffer
   __sso_allocator
   __std_stream
   __string
   __threading_support
   __tree
   __tuple
   __undef_macros
+  __undef_macros_private
   algorithm
   any
   array
   atomic
   barrier
   bit
   bitset
   cassert
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/__bits

 
 #include <__config>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #pragma GCC system_header
 #endif
 
 _LIBCPP_PUSH_MACROS
-#include <__undef_macros>
+#include <__undef_macros_private>
 
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
 #ifndef _LIBCPP_COMPILER_MSVC
 
 inline _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR
 int __libcpp_ctz(unsigned __x)           _NOEXCEPT { return __builtin_ctz(__x); }

libcxx/include/__memory/allocator_traits.h

 // -*- C++ -*-
 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef _LIBCPP___MEMORY_ALLOCATOR_TRAITS_H
 #define _LIBCPP___MEMORY_ALLOCATOR_TRAITS_H
 
 #include <__config>
 #include <__memory/base.h>
 #include <__memory/pointer_traits.h>
-#include <type_traits>
+#include <limits>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #pragma GCC system_header
 #endif
 
 _LIBCPP_PUSH_MACROS
 #include <__undef_macros>
 

libcxx/include/__memory/pointer_traits.h

 // -*- C++ -*-
 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef _LIBCPP___MEMORY_POINTER_TRAITS_H
 #define _LIBCPP___MEMORY_POINTER_TRAITS_H
 
 #include <__config>
-#include <type_traits>
+#include <__memory/base.h>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #pragma GCC system_header
 #endif
 
 _LIBCPP_PUSH_MACROS
 #include <__undef_macros>
 

libcxx/include/__memory/utilities.h

 // -*- C++ -*-
 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef _LIBCPP___MEMORY_UTILITIES_H
 #define _LIBCPP___MEMORY_UTILITIES_H
 
 #include <__config>
 #include <__memory/allocator_traits.h>
-#include <cstddef>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #pragma GCC system_header
 #endif
 
 _LIBCPP_PUSH_MACROS
 #include <__undef_macros>
 

libcxx/include/__sso_allocator

 // -*- C++ -*-
 //===----------------------------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef _LIBCPP___SSO_ALLOCATOR
 #define _LIBCPP___SSO_ALLOCATOR
 
 #include <__config>
-#include <memory>
-#include <new>
 #include <type_traits>
+#include <stddef.h>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #pragma GCC system_header
 #endif
 
 _LIBCPP_BEGIN_NAMESPACE_STD
 
 template <class _Tp, size_t _Np> class _LIBCPP_HIDDEN __sso_allocator;

libcxx/include/__undef_macros_private

+// -*- C++ -*-
+//===------------------------ __undef_macros ------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+
+#ifdef min
+#if !defined(_LIBCPP_DISABLE_MACRO_CONFLICT_WARNINGS)
+#if defined(_LIBCPP_WARNING)
+_LIBCPP_WARNING("macro min is incompatible with C++.  Try #define NOMINMAX "
+                "before any Windows header. #undefing min")
+#else
+#warning: macro min is incompatible with C++.  #undefing min
+#endif
+#endif
+#undef min
+#endif
+
+#ifdef max
+#if !defined(_LIBCPP_DISABLE_MACRO_CONFLICT_WARNINGS)
+#if defined(_LIBCPP_WARNING)
+_LIBCPP_WARNING("macro max is incompatible with C++.  Try #define NOMINMAX "
+                "before any Windows header. #undefing max")
+#else
+#warning: macro max is incompatible with C++.  #undefing max
+#endif
+#endif
+#undef max
+#endif

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 vector {
     export *
   }
   module version {
     header "version"
     export *
   }
 
   // FIXME: These should be private.
-  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 __memory_pointer_traits { header "__memory/pointer_traits.h" export * }
+  module __memory_allocator_traits { header "__memory/allocator_traits.h" export * }
+  module __memory_base { header "__memory/base.h" export * }
+  module __memory_utilities { header "__memory/utilities.h" export * }
 
   module experimental {
     requires cplusplus11
 
     module algorithm {
       header "experimental/algorithm"
       export *
     }

libcxx/include/module.private.modulemap

+module std_Private {
+
+  module __bits { header "__bits" export * }
+
+  module __undef_macros_private { header "__undef_macros_private" export * }
+}