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

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

In D126694#3577815 <https://reviews.llvm.org/D126694#3577815>, @iains wrote:

> In D126694#3576853 <https://reviews.llvm.org/D126694#3576853>, @ChuanqiXu wrote:
>
>> In D126694#3576602 <https://reviews.llvm.org/D126694#3576602>, @iains wrote:
>>
>>> @ChuanqiXu - I changed the module ownership name to "ModuleDiscardable" - because, while we are permitted to discard these, we might choose not to (to give your better diagnostics) - but we still need to treat them as non-reachable and non-visible.  Please could you examine what is happening with `Modules/explicitly-specialized-template.cpp` where there a multiple error messages for each (intentionally) failing line .. add the test from the std.
>>
>> I've looked into this. The multiple duplicated diagnostic message is a legacy problem. I tried to fix it but it is hard (change it would break many other tests). To filter out the redundant duplicated diagnostic message, you could use '+1' suffix after `expected-error`. For example: https://github.com/llvm/llvm-project/blob/16ca490f450ea3ceaeda92addd8546967af4b2e1/clang/test/Modules/cxx-templates.cpp#L215-L232 BTW, after I filtered out the duplicated diagnostic message,
>
> This was not the problem in this particular case - but (for reference) is there an issue for the duplicated diagnostics? - that seems not very user-friendly, especially since C++ emits a lot of long diagnostics already.

I failed to find one and I filed one at: https://github.com/llvm/llvm-project/issues/56024. Hope this helps.

>>> @rsmith
>>>  this seems like an awful lot of work that has to be done for every decl we mark used in the module purview - we cannot even short-cut ones that are not in the GMF, since the provisions of [module.global.frag] p3.5 seem to allow this to happen for indirect reaching.  I wonder if I misread the std.
>>
>> Yeah, and I am not sure what is better idea here. I tried to implement GMF decl elision before. My first idea is similar to your first revision. But I tried to implement indirect discarding that time. So I chose to traverse the decls in GMF until a fixed point when we handle the end of the TU. It looks bad clearly (the time complexity is not satisfying). So I didn't post it up.
>>
>>> I am also wondering what is supposed to happen when an interface makes a type reachable, but that type is not visible to the implementation .. it seems to mean that interfaces would have to add using declarations for each such type.
>>
>> @rsmith should be the best to answer here. But I am trying to answer it. If  'the implementation' means the implementation unit, I think the type is naturally visible to the implementation unit. We could find the example in: module.interface-example5 <https://eel.is/c++draft/module.interface#example-5> .
>
> That is not the problem I am describing - which more like:
>
> type.h:
> typedef int MyInt;
>
> mod-interface.cpp:
> module;
> #include "type.h"
> export module A;
>
> MyInt foo (); // makes MyInt reachable, but not visible.
>
> mod-imp.cpp:
> module A;
>
> MyInt foo () { return 42; } // will not compile because the type declaration is not visible.
>
>   I am told that this is the intention .. it just seems a wee bit odd.

Oh, it makes sense now. We could include  "type.h" in the implementation unit so it wouldn't matter I think.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126694