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

[Mark de Wever via Phabricator via libcxx-commits]

Mordante added a comment.

In D156177#4535832 <https://reviews.llvm.org/D156177#4535832>, @JDevlieghere wrote:

> In D156177#4535652 <https://reviews.llvm.org/D156177#4535652>, @Mordante wrote:
>
>> I understand it's not great to have the LLDB build broken.
>
> Not only is it not great, it's not in accordance with LLVM's developer policy: https://llvm.org/docs/DeveloperPolicy.html#patch-reversion-policy. I could have reverted the original patch. Instead, we're trying work together here towards a constructive solution that works for both of our projects.

I'm aware of the policy. However I feel somewhat pressured to approve patch that will not work for some libc++ users. Using a modular build with locales disabled will not work. So landing the patch in its current state can break other users. I've spoken with @iana and we will postpone the generation part, that can be done in a separate patch. That makes the finishing this patch a lot easier. (Since we don't add headers in the release this will not accidentally give issues.)

>> It would be really great if the LLDB team could help libc++ to get LLDB working in the libc++ pre-commit CI to prevent breakage.
>
> We did, and  D133621 <https://reviews.llvm.org/D133621> was the result of that. I don't know why it was never merged?

Good question. @ldionne is out of the office, I'll ask him when he's back.

>> Both @ldionne and I have reached out in the past to get assistance, but that did not result in a working LLDB in the CI. I don't recall the exact issues I ran into last time I tried, which I think is over a year ago.
>
> Who did you reach out to? This is the first time I hear about that.

I don't remember it has been over a year. I tried to find the patch in Phabricator but I can't find it.

We spend quite a bit of effort in libc++ to prevent breakage by running an extensive pre-commit CI (±60 jobs). So it's a bit frustrating that we haven't been able to get LLDB working and again discover breakage after a patch has landed. If we need assistance to get LLDB running in the CI can we reach out to you?



================
Comment at: libcxx/docs/Contributing.rst:50
 
+  - Did you add it to ``include/__std``?
   - Did you add it to ``include/module.modulemap.in``?
----------------
Since we're going to add a script, so this can be removed.


================
Comment at: libcxx/include/__std:49
+#include <list>
+#include <locale>
+#include <map>
----------------
Please check which headers can be included in the libc++ build. Including this header will fail when locales are disabled.
`libcxx/utils/libcxx/test/header_information.py` has this information.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156177