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

Chuanqi Xu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Mar 14 02:47:05 PDT 2023


ChuanqiXu added a comment.

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

> In D145965#4192072 <https://reviews.llvm.org/D145965#4192072>, @ChuanqiXu wrote:
>
>> In D145965#4192051 <https://reviews.llvm.org/D145965#4192051>, @iains wrote:
>>
>>> 
>
>
>
>>>
>>>
>>>
>>> The checks for internal-linkage symbols and the improvements to diagnostics do not depend on **how** `Sema::isModuleUnitOfCurrentTU` is implemented, so that I think we could fix these problems and then deal with the refactoring later.
>>>
>>> In either case, I do not have much time to work more on this right now.
>>
>> I'll try to make it.
>
> Perhaps then, we can split out the changes to `Sema::isModuleUnitOfCurrentTU` and see how much of the functionality is still working (with the intent that the full fix will come along  later).
>
> So if this is split into two
>
> 1. the fix to `Sema::isModuleUnitOfCurrentTU`  (which you will be updating) and
> 2. the uses of that to fix the lookups and diagnostics?
>
> (I can use both as a temporary fix ..  but 2 could be applied with the existing `isModuleUnitOfCurrentTU` implementation).

I feel it may be better to wait for me to have an initial experiment. On the one hand, if we can make it relatively quickly, we can save some time. On the other hand, now I understand that removing codes is much harder than adding codes... So let's wait for some time. The next release is in the end of July, so we have plenty time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145965



More information about the cfe-commits mailing list