[libcxx-commits] [PATCH] D144994: [Draft][libc++][modules] Adds std module.

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added subscribers: jwakely, CaseyCarter.
ldionne added inline comments.


================
Comment at: libcxx/utils/use_modules_in_test.py:1
+#!/usr/bin/env python
+# ===------------------------------------------------------------------------------===##
----------------
Let's extract this to its own patch.


================
Comment at: libcxx/utils/use_modules_in_test.py:159
+UNSUPPORTED_TESTS = [
+    "libcxx/test/std/utilities/meta/meta.unary/meta.unary.prop/is_literal_type.pass.cpp",
+]
----------------
Could we instead use this annotation in the test itself, with a relevant comment?

```
// UNSUPPORTED: use_module_std && !(c++11 || c++14 || c++17)
```

Or actually, we can use just

```
// UNSUPPORTED: use_module_std
```

since we never run with `use_module_std` in C++11/14/17.


================
Comment at: libcxx/utils/use_modules_in_test.py:169-176
+DISABLE_DEPRECATION_WARNINGS_REGEXES = [
+    re.compile(
+        r"//\s*ADDITIONAL_COMPILE_FLAGS:.*-D_LIBCPP_DISABLE_DEPRECATION_WARNINGS"
+    ),
+    re.compile(r"//\s*ADDITIONAL_COMPILE_FLAGS:.*-D_LIBCPP_ENABLE_ASSERTIONS"),
+    re.compile(r"//\s*ADDITIONAL_COMPILE_FLAGS:.*-D_LIBCPP_HAS_NO_UNICODE"),
+    re.compile(r"//\s*ADDITIONAL_COMPILE_FLAGS:.*-fsized-deallocation"),
----------------
The fact that we need to disable these tests is quite interesting. I think it surfaces a pretty big problem with modules (as a language feature). Today, we use macros and compiler flags to customize the behavior of the library on a per translation-unit basis. That is an extremely important property of some aspects of the library (*). If we want everything to transition to modules (and we do), we need a way to satisfy those use cases. I don't mind whether that's done by defining macros, via some compiler flags or some other way, but those use cases are valid and we need to figure out how we're going to support them. Otherwise, we'll be left with large parts of our user base that can't switch over to C++ modules because they don't support some required use cases.

(*) For example, we use `_LIBCPP_ENABLE_ASSERTIONS` to control whether assertions are enabled or not on a per-TU basis. This is extremely important: when converting code bases to assertions, users might want to prevent assertions in some performance critical parts of the code, while still enabling assertions everywhere else. This approach allows for an incremental adoption of the feature and that's an important aspect of it being deployable in the real world.

I would like us to discuss avenues we could use to satisfy these use cases. For example, could we ship multiple "versions" of the library, each configured differently, and then the compiler would use a different version based on the compiler flags it is passed? I think pretty much all vendors are going to have similar questions, and that's going to severely limit how useful C++ modules can be in the real world. @jwakely @CaseyCarter I assume that's also something that you folks have thought about?

CC @ChuanqiXu 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144994