[PATCH] D145965: [C++20][Modules] Fix incorrect visibilities in implementation units.

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

In D145965#4194514 <https://reviews.llvm.org/D145965#4194514>, @iains wrote:

> In D145965#4192051 <https://reviews.llvm.org/D145965#4192051>, @iains wrote:
>
>> 
>
>
>
>> I was thinking at one stage to add an 'Implementation' module Kind, but at the moment I do not think it is worth extending the size of the ModuleKind enum bit field for this (since we now have used up all the entries with the new 'ExplicitGlobalModuleFragment' case).  That also has a large impact also for relatively few uses.
>
> I have changed my mind here.
> It seems that we are getting confusing decl context entries without having a proper module for the implementation (even though we will never write such a module to a PCM, since it is not importable).
>
> I was wondering if it would be possible to avoid having separate values for PartitionInterface/Implementation (since they both have a BMI - it is only a question of whether that can be re-exported).  We would have to add one bit to the PCM to deal with that, so that we might as well extend the ModuleKind enum to 4 bits.
>
> So .. please wait for one or two days, I will try to refresh the implementation patch (D126959 <https://reviews.llvm.org/D126959>) and see if that resolves some of the issues I am seeing with `P1815`
>
> (this does not alter the situation that you are going to revisit the `isModuleUnitOfCurrentTU` if that is needed - but it might not be; since having a separate module for the impl. will avoid some of the ambiguities anyway).
>
> Additional Note:  we also have a situation in the lookups for dependent names during template instantiation that the visibility of decls can need to be assessed in a different module context from the current module.  So that we might also need to have a `areTheseModulesFromTheSameTU(Module *A. Module *B)`

Yeah. What I had in mind is to remove `ModulePartitionInterface` and `ModulePartitionImplementation` in ModuleKinds and rename `ModuleInterfaceUnit` to `NamedModules`. And we can introduce 2 bits for IsPartition and IsImplementation. Then we can introduce ImplementationUnit without any loss.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145965