[libcxx-commits] [PATCH] D156907: [libc++][modules] Removes the module partitions.

Wed Aug 2 20:38:12 PDT 2023

ChuanqiXu added a comment.

In D156907#4556195 <https://reviews.llvm.org/D156907#4556195>, @philnik wrote:

> In D156907#4556159 <https://reviews.llvm.org/D156907#4556159>, @ChuanqiXu wrote:
>
>> In D156907#4556138 <https://reviews.llvm.org/D156907#4556138>, @philnik wrote:
>>
>>> I'm a bit concerned about the maintainability of this. AFAICT this change makes it basically impossible for tooling to parse the modules correctly. Would it be possible to have one partition that includes everything and have other modules do the actual exports (or something similar)? That should allow tooling to parse everything correctly and we don't have the problem that we have to parse a lot of code multiple times. Another option would be to just have a single huge file. I haven't looked at everything, but the total size would probably be < 5k lines, which wouldn't even make it the largest file we have in the library.
>>
>> While the overhead to introduce another partition should be small, I don't understand what's the problem for tools to parse the modules correctly now?
>
> The problem is that the files aren't self-contained, so a tool parsing only a single file doesn't know that other stuff happens to already be included. That's a huge problem for IDEs.

IIUC, you're saying when you're opening files like `modules/std/vector.inc`, the IDE/code intelligence can't tell you any thing. I am not sure if this is a problem. If yes, it may be a solution to create a huge file called `std.cppm`. And we can't put all the include in a partition without an `export` and export all of them in the primary module interface. Since the entities in the global module fragments are not visible to other module units even if they are in the same module.

>>> w.r.t. back-porting this it's a definite no from me, since this is an improvement of the library. Just like we shouldn't back-port other performance improvements without a really good reason. This feature is also still experimental. There is always a next release in a few months.
>>
>> I think 8.x compilation speedup and 10.x size reduction may be a good reason.
>
> I don*t think so. We have these kinds of speedups all the time. I have like a dozen patches locally that reach these kinds of speedups (some even 300x) for algorithms, but we shouldn't back-port them because they are improvements; they aren't fixing a regression.

Of course it is up to the libcxx developers. I just want to express my feelings as a user or a c++ fans. And I feel it is different from the speedups for algorithms. Since the std modules are big whole thing. I mean, the big improvement on a small subsystem may be small for the overall system. Also many improvements depend on the input. It is usual that we made some improvements for a (kind of) specific input(s) but without affecting the overall system. However, for the use of std module, there is only one input. So I feel it is pretty important.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156907