[PATCH] D122119: [C++20][Modules] Adjust handling of exports of namespaces and using-decls.

Chuanqi Xu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Mar 22 19:51:25 PDT 2022


ChuanqiXu added a comment.

In D122119#3400032 <https://reviews.llvm.org/D122119#3400032>, @urnathan wrote:

> In D122119#3398949 <https://reviews.llvm.org/D122119#3398949>, @ChuanqiXu wrote:
>
>> 
>
>
>
>> The first feeling I saw the change is that not every C++ programmer knows about linkage. OK, it depends on the environment really and every one might has their own opinion.
>
> You may be confusing object-file linkage with the linkage concepts of C++, which are specified in [basic.link]?  Sadly we have to live with the overloaded term.
>
>> Another thought is that 10.2.6 (http://eel.is/c++draft/module.interface#6) doesn't talk anything about linkage:
>>
>>> A redeclaration of an entity X is implicitly exported if X was introduced by an exported declaration; otherwise it shall not be exported.
>>
>> So it looks like confusing to talk about linkage this time. In my imagination, there might be a such situation:
>>
>> A programmer met the error when he tries to export a redeclaration which is internal linkage (maybe a simple const variable). Then the message told him the internal linkage is not allowed to re-export. Then he removes the const specifier. Now he meets the error again. It tells that we couldn't export redeclaration which is module linkage. I guess he would feel bad. Then he might try to export the first declaration to get passed. However, the `const` specifier is lost in the case. And in the current message, I guess he would add export to the first declaration directly after he reads the message.
>
> further, with attachment, the original error 'cannot export as previous was not exported' is not correct in general.  Consider:
>
> module;
> // Pretend this is from #include, ok?
> void Foo ();
> module bob;
> extern "C++" export void Foo ();  // can export even though prior was not exported

I think the example is invalid since it violates [[ http://eel.is/c++draft/module.interface#6 | [module.interface]p6 ]] explicitly. If this is intended behavior or by design, we should change the wording.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122119



More information about the cfe-commits mailing list