[PATCH] D126694: [C++20][Modules] Initial implementation of GMF decl elision.

[Iain Sandoe via Phabricator via cfe-commits]

iains added a comment.

In D126694#3565417 <https://reviews.llvm.org/D126694#3565417>, @ChuanqiXu wrote:

> In D126694#3564629 <https://reviews.llvm.org/D126694#3564629>, @iains wrote:
>
>> the first failure is like this:
>>
>>   x-h.h:
>>   struct A {
>>     friend int operator+(const A &lhs, const A &rhs) {
>>       return 0;
>>     }
>>   };
>>   
>>   X.cpp:
>>   module;
>>   #include "x-h.h"
>>   export module X;
>>   
>>   export using ::A;
>>
>> This does not mark *anything* in the GMF (x-h.h) as 'used', and so everything there is unreachable (and hence the fails).
>> I.e `export using ::A;` neither marks A as used or referenced.
>> I am not sure if we are supposed to special-case this - since `https://eel.is/c++draft/module#global.frag-3.6` explicitly says "In this determination, it is unspecified .. whether `using-declaration, ...  is replaced by the declarations they name prior to this determination,`
>> so .. not about how to proceed with this one at present; 
>> edit:  but it seems most reasonable to make it behave as if A was itself  exported.
>
> I highly recommend we should mark A here. Maybe we need other interfaces than markDeclUsed and setDeclReferenced. If we don't support this, we couldn't use modules like https://github.com/alibaba/async_simple/blob/CXX20Modules/third_party_module/asio/asio.cppm. This manner is important to use C++20 modules before the build system is ready. Also, I think it is an important tool to implement C++23's std modules. So we should support it.

Actually, after thinking some more, what seems to be wrong here is that we should be making the exported item "VisibleIfImported" .. which is not being done - I guess this was a bug already and it has been exposed by the recent changes in the module ownership processing.  I will next take a look at that (and the other comments).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126694