[libc++][modules] Mark as implemented. #90091
Conversation
The feature has been implemented in LLVM 18 as an experimental feature. This marks the paper as complete and sets the feature-test macro. Implements - P2465R3 Standard Library Modules std and std.compat Fixes: llvm#89579
llvmbot
added
the
libc++
|
@llvm/pr-subscribers-libcxx Author: Mark de Wever (mordante) ChangesThe feature has been implemented in LLVM 18 as an experimental feature. This marks the paper as complete and sets the feature-test macro. Implements
Fixes: #89579 Full diff: https://github.com/llvm/llvm-project/pull/90091.diff 5 Files Affected:
diff --git a/libcxx/docs/FeatureTestMacroTable.rst b/libcxx/docs/FeatureTestMacroTable.rst
index 3197d2cd1b271c..858a4ebd27a07e 100644
--- a/libcxx/docs/FeatureTestMacroTable.rst
+++ b/libcxx/docs/FeatureTestMacroTable.rst
@@ -340,6 +340,8 @@ Status
---------------------------------------------------------- -----------------
``__cpp_lib_mdspan`` ``202207L``
---------------------------------------------------------- -----------------
+ ``__cpp_lib_modules`` ``202207L``
+ ---------------------------------------------------------- -----------------
``__cpp_lib_move_only_function`` *unimplemented*
---------------------------------------------------------- -----------------
``__cpp_lib_optional`` ``202110L``
diff --git a/libcxx/docs/Status/Cxx23Papers.csv b/libcxx/docs/Status/Cxx23Papers.csv
index 01387a404f5d67..d29b4a6bdbc1ce 100644
--- a/libcxx/docs/Status/Cxx23Papers.csv
+++ b/libcxx/docs/Status/Cxx23Papers.csv
@@ -75,7 +75,7 @@
"`P2445R1 <https://wg21.link/P2445R1>`__","LWG","``forward_like``","July 2022","|Complete|","16.0"
"`P2446R2 <https://wg21.link/P2446R2>`__","LWG","``views::as_rvalue``","July 2022","|Complete|","16.0","|ranges|"
"`P2460R2 <https://wg21.link/P2460R2>`__","LWG","Relax requirements on ``wchar_t`` to match existing practices","July 2022","",""
-"`P2465R3 <https://wg21.link/P2465R3>`__","LWG","Standard Library Modules ``std`` and ``std.compat``","July 2022","",""
+"`P2465R3 <https://wg21.link/P2465R3>`__","LWG","Standard Library Modules ``std`` and ``std.compat``","July 2022","|Complete|","18.0",""
"`P2467R1 <https://wg21.link/P2467R1>`__","LWG","Support exclusive mode for ``fstreams``","July 2022","|Complete|","18.0",""
"`P2474R2 <https://wg21.link/P2474R2>`__","LWG","``views::repeat``","July 2022","|Complete|","17.0","|ranges|"
"`P2494R2 <https://wg21.link/P2494R2>`__","LWG","Relaxing range adaptors to allow for move only types","July 2022","|Complete|","17.0","|ranges|"
diff --git a/libcxx/include/version b/libcxx/include/version
index 0ed77345baa71d..09d3c5ed384db4 100644
--- a/libcxx/include/version
+++ b/libcxx/include/version
@@ -465,6 +465,7 @@ __cpp_lib_within_lifetime 202306L <type_traits>
# define __cpp_lib_ios_noreplace 202207L
# define __cpp_lib_is_scoped_enum 202011L
# define __cpp_lib_mdspan 202207L
+# define __cpp_lib_modules 202207L
// # define __cpp_lib_move_only_function 202110L
# undef __cpp_lib_optional
# define __cpp_lib_optional 202110L
diff --git a/libcxx/test/std/language.support/support.limits/support.limits.general/version.version.compile.pass.cpp b/libcxx/test/std/language.support/support.limits/support.limits.general/version.version.compile.pass.cpp
index 3ec548f56cea1d..09f471693acf0c 100644
--- a/libcxx/test/std/language.support/support.limits/support.limits.general/version.version.compile.pass.cpp
+++ b/libcxx/test/std/language.support/support.limits/support.limits.general/version.version.compile.pass.cpp
@@ -143,6 +143,7 @@
__cpp_lib_math_special_functions 201603L [C++17]
__cpp_lib_mdspan 202207L [C++23]
__cpp_lib_memory_resource 201603L [C++17]
+ __cpp_lib_modules 202207L [C++23]
__cpp_lib_move_iterator_concept 202207L [C++20]
__cpp_lib_move_only_function 202110L [C++23]
__cpp_lib_node_extract 201606L [C++17]
@@ -731,6 +732,10 @@
# error "__cpp_lib_memory_resource should not be defined before c++17"
# endif
+# ifdef __cpp_lib_modules
+# error "__cpp_lib_modules should not be defined before c++23"
+# endif
+
# ifdef __cpp_lib_move_iterator_concept
# error "__cpp_lib_move_iterator_concept should not be defined before c++20"
# endif
@@ -1583,6 +1588,10 @@
# error "__cpp_lib_memory_resource should not be defined before c++17"
# endif
+# ifdef __cpp_lib_modules
+# error "__cpp_lib_modules should not be defined before c++23"
+# endif
+
# ifdef __cpp_lib_move_iterator_concept
# error "__cpp_lib_move_iterator_concept should not be defined before c++20"
# endif
@@ -2597,6 +2606,10 @@
# endif
# endif
+# ifdef __cpp_lib_modules
+# error "__cpp_lib_modules should not be defined before c++23"
+# endif
+
# ifdef __cpp_lib_move_iterator_concept
# error "__cpp_lib_move_iterator_concept should not be defined before c++20"
# endif
@@ -3896,6 +3909,10 @@
# endif
# endif
+# ifdef __cpp_lib_modules
+# error "__cpp_lib_modules should not be defined before c++23"
+# endif
+
# ifndef __cpp_lib_move_iterator_concept
# error "__cpp_lib_move_iterator_concept should be defined in c++20"
# endif
@@ -5354,6 +5371,13 @@
# endif
# endif
+# ifndef __cpp_lib_modules
+# error "__cpp_lib_modules should be defined in c++23"
+# endif
+# if __cpp_lib_modules != 202207L
+# error "__cpp_lib_modules should have the value 202207L in c++23"
+# endif
+
# ifndef __cpp_lib_move_iterator_concept
# error "__cpp_lib_move_iterator_concept should be defined in c++23"
# endif
@@ -7139,6 +7163,13 @@
# endif
# endif
+# ifndef __cpp_lib_modules
+# error "__cpp_lib_modules should be defined in c++26"
+# endif
+# if __cpp_lib_modules != 202207L
+# error "__cpp_lib_modules should have the value 202207L in c++26"
+# endif
+
# ifndef __cpp_lib_move_iterator_concept
# error "__cpp_lib_move_iterator_concept should be defined in c++26"
# endif
diff --git a/libcxx/utils/generate_feature_test_macro_components.py b/libcxx/utils/generate_feature_test_macro_components.py
index f2b8d55c0e11b0..423acbca82b938 100755
--- a/libcxx/utils/generate_feature_test_macro_components.py
+++ b/libcxx/utils/generate_feature_test_macro_components.py
@@ -830,6 +830,11 @@ def add_version_header(tc):
"test_suite_guard": "!defined(_LIBCPP_VERSION) || _LIBCPP_AVAILABILITY_HAS_PMR",
"libcxx_guard": "_LIBCPP_AVAILABILITY_HAS_PMR",
},
+ {
+ "name": "__cpp_lib_modules",
+ "values": {"c++23": 202207},
+ "headers": [],
+ },
{
"name": "__cpp_lib_move_iterator_concept",
"values": {"c++20": 202207},
|
There was a problem hiding this comment.
IIRC part the reasoning for adding modules unconditionally was that vendors don't have to actually install them. Them change seems to me like it forces vendors to install them, since we claim support through the FTM. I'm also not sure we should already claim support given that, AFAICT, it's still very much an experimental feature which isn't supported in a lot of cases. Third and last: when we set the macro, should we also set it in C++20?
There was a problem hiding this comment.
I think we'd need to make the FTM conditional on whether modules are installed (via LIBCXX_INSTALL_MODULES). In the long term, I think this also means that we'll want to simply drop LIBCXX_INSTALL_MODULES and always install them when we install the libc++ headers, i.e. LIBCXX_INSTALL_HEADERS would imply that we install the modules files as well.
libcxx/docs/Status/Cxx23Papers.csv
Outdated
| @@ -75,7 +75,7 @@ | |||
| "`P2445R1 <https://wg21.link/P2445R1>`__","LWG","``forward_like``","July 2022","|Complete|","16.0" | |||
| "`P2446R2 <https://wg21.link/P2446R2>`__","LWG","``views::as_rvalue``","July 2022","|Complete|","16.0","|ranges|" | |||
| "`P2460R2 <https://wg21.link/P2460R2>`__","LWG","Relax requirements on ``wchar_t`` to match existing practices","July 2022","","" | |||
| "`P2465R3 <https://wg21.link/P2465R3>`__","LWG","Standard Library Modules ``std`` and ``std.compat``","July 2022","","" | |||
| "`P2465R3 <https://wg21.link/P2465R3>`__","LWG","Standard Library Modules ``std`` and ``std.compat``","July 2022","|Complete|","18.0","" | |||
There was a problem hiding this comment.
I think this should be 19.0 since basically no vendor will have shipped these modules files in 18.0. So marking 18.0 is accurate from the standpoint of the library work that was done, but not useful for any users (and in fact rather confusing).
There was a problem hiding this comment.
I'm not sure whether or not vendors enabled it. I know it's enabled on Debian. However I don't feel strongly about 18 or 19 so I changed it.
|
IMO in general we document what we do not what vendors do. For example, we mark features as implemented even when Apple decides not to make them available on backdeployment targets. (I understand why Apple does not want to do that.). Especially in this case; we have no control whether a vendor does or does not ship the installed module. Just testing for the CMake flag does tell us what the vendor does. So I'm happy to change that, but IMO then we should discuss what we consider as implemented and when we can mark things as implemented. |
Just to clarify, Apple doesn't "make features unavailable on backdeployment targets". It's simply that new features that require dylib support can't work on older targets that don't have said support and we happen to care enough about users to have the availability machinery to let users know at compile-time (instead of crashing at load-time). Any system that uses shared libraries and ships libc++ as part of the operating system would experience exactly the same limitations. Just clarifying for readers cause your initial statement made it look a bit like Apple deliberately made things unsupported on older platforms, which is not true and would be a questionable practice.
I think this is a bit different though: this setting was basically an "experimental" feature disabled by default and only existing for development purposes until now. But in LLVM 19, we are comfortable with getting this used a bit more widely, and so we can enable the CMake installation of modules by default, and ping vendors to let them know. Then we're giving them the choice of either shipping modules or not. But I think it's difficult to argue that we considered C++20 modules as implemented in LLVM 18 given the (lack of) communication we did about it, and how hidden we kept the setting. |
I dislike that idea; installation does not mean the vendor ships it at all. So we don't have a good way to detect whether modules are really supported. Several FTM depend on the dylib, these are also available unconditionally even when vendors can turn of
Thanks. It was not my intention to suggest malice on Apple's side. It was just the first example that came to mind where feature-test macros are not always correct. |
1. __cpp_lib_modules now is defined. llvm/llvm-project#90091 2. clang-cl supports -x option for all languages. llvm/llvm-project#89772 3. Header units allow non-inline external static members definitions. llvm/llvm-project#98309 4. winsysroot alias to the GNU driver: -Xmicrosoft-windows-sys-root. llvm/llvm-project#94731 5. clang-cl gained options for compiling C++ modules. llvm/llvm-project#98761 6. fix unused argument warning for /std:c++20 when compiling c++ modules. llvm/llvm-project#99300
The feature has been implemented in LLVM 18 as an experimental feature. This marks the paper as complete and sets the feature-test macro.
Implements
Fixes: #89579