[libcxx-commits] [PATCH] D156177: [libc++][Modules] Recreate the top level `std` clang module

[Ian Anderson via Phabricator via libcxx-commits]

iana marked 2 inline comments as done.
iana added inline comments.


================
Comment at: libcxx/include/__std_clang_module:185
+
+#if _LIBCPP_STD_VER >= 14
+#  ifndef _LIBCPP_HAS_NO_THREADS
----------------
Mordante wrote:
> Typically we don't use the language version guards. They also seem to be incomplete. For example `<print>` is C++23 only.
> I don't mind it for this patch, it's more a remark to keep in mind for the generator patch.
> 
> Including `<print>` in C++03 does compile and not give diagnostics.
> Including `<locale>` with `_LIBCPP_HAS_NO_LOCALIZATION` will trigger an `#error "foo"` in the pre-processor.
> That's not clear from the annotations in the python file since that is used for the test where we indeed need to disable tests in older language versions.
I'll see if I can ignore the versions in 


================
Comment at: libcxx/include/__std_clang_module:185
+
+#if _LIBCPP_STD_VER >= 14
+#  ifndef _LIBCPP_HAS_NO_THREADS
----------------
iana wrote:
> Mordante wrote:
> > Typically we don't use the language version guards. They also seem to be incomplete. For example `<print>` is C++23 only.
> > I don't mind it for this patch, it's more a remark to keep in mind for the generator patch.
> > 
> > Including `<print>` in C++03 does compile and not give diagnostics.
> > Including `<locale>` with `_LIBCPP_HAS_NO_LOCALIZATION` will trigger an `#error "foo"` in the pre-processor.
> > That's not clear from the annotations in the python file since that is used for the test where we indeed need to disable tests in older language versions.
> I'll see if I can ignore the versions in 
Ok, I guess we'll need to add something to header_information.py for codifying the include rules. Either that or the script could just try to include each header with each mode and see if it errors or not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156177