[PATCH] D118586: [C++20][Modules][3/8] Initial handling for module partitions.

Mon Feb 21 22:40:23 PST 2022

ChuanqiXu added a comment.

In D118586#3336865 <https://reviews.llvm.org/D118586#3336865>, @iains wrote:

> In D118586#3336612 <https://reviews.llvm.org/D118586#3336612>, @ChuanqiXu wrote:
>
>> (May off the patch)
>
> I am not sure what you mean here.

I mean this comment is not related to the patch. So it doesn't mean to block the patch landing.

>> BTW, I know you also work on GCC frontend. I want to consultant what's your opinion to uniform the command line options between Clang and GCC. Now it looks totally different. I feel it would be a problem for clang. Since the command line between clang and gcc are basically compatible so that user who decide to use clang could turn back to use gcc whenever he want. But I am worrying that it would be a harder decision for users if them are going to use clang. Since the cost to use GCC again would be higher.
>
> We have the same objective: to keep user-facing options and functionality the same between clang and GCC (easier for both user and for build systems).
> The content of this patch does not alter that. BTW GCC also keeps the module name as "Primary:Parition" internally and user-facing output.  //The module name in C++20 modules has no fixed mapping to the BMI/CMI filename on disk,.//

I just tested the behavior of GCC. When I compile a partition interface unit (let's name is `mod:part`), it would generate a file named `mod-part,gcm` under the directory `gcm.cache`. So I feel what GCC does here is more aggressive, it doesn't only choose the separator '-'. It would create an cache directory by default. (I am not blaming it. I like the style too. Otherwise, we need to sore CMI to a place by hand). What I want to say here is that GCC would replace '-' to ':' when maps to the filesystem. I am OK to not implement this in the series patch. But I hope we could get in consensus that we should implement this.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118586