[libcxx-commits] [PATCH] D119036: [libc++] Fix modules and benchmarks CI builds when incomplete features are disabled

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added a comment.

In D119036#3311492 <https://reviews.llvm.org/D119036#3311492>, @aprantl wrote:

> Are you suggesting we drop support for building libcxx without local submodule visibility? I'm not sure that's an acceptable regression.

Let's put it that way: libc++ already doesn't support it, because it doesn't work and we don't test it. From our side, the situation is that we've done a huge amount of work to support modules because we knew a few projects were using it (LLDB being the biggest one I'm aware of), despite that being rather painful and very light on return on investment. Modules in Clang have some bugs that have been biting us, making things difficult. The only thing that's on our radar is whatever we test, and at the moment it's modules with local submodule visibility.

If there is a reason for us to support modules without local submodule visibility, then yes, we can look into supporting it, however I think the correct order is:

1. Make sure LLDB uses an officially supported and tested way to build and use libc++ (which you partially address in 547a667ceeb6 <https://reviews.llvm.org/rG547a667ceeb60dca5447e5bc09165a52b22925eb>, thanks for that)
2. Tell us about the fact that you'd like modules to be supported without local submodule visibility (that's clear now)
3. Then we make any changes necessary to support it
4. LLDB switches to modules without local submodule visibility

I think the friction here comes from the fact that LLDB (1) consciously decides to live on the edge by using a ToT libc++, and (2) doesn't give us a way to put LLDB on our radar by providing a pre-commit CI bot that would run the data formatters. This leads to this situation where we broke you, but we were not even aware that you *could* be broken. I thought the story of breaking LLDB data formatters (which used to happen frequently) was completely done because we added the right modules CI bot, but it seems like there was still a difference in the flag.

Anyway I'm immediately going to look into what's necessary to support modules without local submodule visibility, but I guess this question is still open:

> If the plan is to not support building libcxx without local submodule visibility, should -fcxx-modules imply -fmodules-local-submodule-visibility ?

I don't know the answer to that. In fact, my initial impression would be that we want to support the "basic" form of modules, i.e. just `-fmodules`. Is there a reason to support more than that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119036