[libcxx-commits] [PATCH] D151030: [libc++][modules] Adds std module cppm files.

[Chuanqi Xu via Phabricator via libcxx-commits]

ChuanqiXu accepted this revision.
ChuanqiXu added a comment.

In D151030#4361219 <https://reviews.llvm.org/D151030#4361219>, @Mordante wrote:

> In D151030#4359443 <https://reviews.llvm.org/D151030#4359443>, @ChuanqiXu wrote:
>
>> I just took a look at it and it looks almost fine. I can't be sure if there is anything missing. But it may be better to decide this by testing instead of by eyes. So this looks good to me.
>
> This has indeed been tested by a test script in D144994 <https://reviews.llvm.org/D144994>. That script has found quite some errors I made. It also helped a lot to update the `.cppm` files after rebasing. It now matches the current implementation status of libc++. Once the rest of the infrastructure lands this script will make sure the CI fails when the headers and the `.cppm` are out of sync.

Got it. So this looks good to me : )



================
Comment at: libcxx/modules/std/vector.cppm:20
+  using std::operator==;
+#if 0 // P1614
+  using std::operator<=>;
----------------
Mordante wrote:
> ChuanqiXu wrote:
> > It looks better if we can use meaningful name like `#ifndef _LIBCPP_HAS_NO_WIDE_CHARACTERS`. 
> I expect these to be short-lived @H-G-Hristov is putting quite some effort in implementing P1614. The `#ifndef _LIBCPP_HAS_NO_WIDE_CHARACTERS` is intended to be forever. I've done similar things for other missing parts of headers or entire headers. For example `print.cppm` exists but libc++ doesn't have `<print>` yet. (That header is being worked on but it hasn't been reviewed yet.)
Yeah, it should be up to you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151030