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

Iain Sandoe via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Feb 21 23:08:31 PST 2022


iains added a comment.

In D118586#3336892 <https://reviews.llvm.org/D118586#3336892>, @ChuanqiXu wrote:

> 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.

I think I am not explaining well enough ...

Please execute "strings  gcm.cache/mod-part,gcm" - you will see that the Module name is "mod:part" in the module file (GCC knows the module as mod:part internally, it is only the on-disk name that is changed).

We //could// choose to use "mod-part.pcm" in our example test cases here, but it would not make any difference to the requirement - the mapping between module name and filename has to be determined by the mechanism that interfaces with the filesystem (we cannot impose that mapping on the module names internally to the compiler - since the mapping is unknown in the general case).

So we would implement a basic interface (in the module loader, I suppose since that is available for jobs without a preprocessor)  that would give pathNameForModuleName(ModuleName) ,... and probably moduleNameForPathName(PathName) ... those would interface to whatever means was used (e.g. ,module mapper, P1184 <https://reviews.llvm.org/P1184> etc).  You could arrange for the default implementation to do the translation you see in GCC (mod:part <=> mod-part.pcm).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118586



More information about the cfe-commits mailing list