Skip to content

[C++20] [Modules] Diagnose exposure for TUlocal entity #112294

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Tracked by #112295
ChuanqiXu9 opened this issue Oct 15, 2024 · 1 comment
Open
Tracked by #112295

[C++20] [Modules] Diagnose exposure for TUlocal entity #112294

ChuanqiXu9 opened this issue Oct 15, 2024 · 1 comment
Labels
clang:modules C++20 modules and Clang Header Modules

Comments

@ChuanqiXu9
Copy link
Member

The wording is at http://eel.is/c++draft/basic.link#14

The simplest example is:

export module a;
static int a = 43;
export inline int func() {
    return a;
}

According to the spec, we should reject the example and say “the TU-local entity ‘a’ is an exposure”. But we didn’t implement this. We would accept the example silently. The practical affect this is, user may get weird diagnostic informations triggered by other places. Or, in the worst case, they may get runtime errors if the static variable have dyanmic initializers.

The reason why I didn’t implement this is, to implement this now, we may need to implement something more complex than the existing mechanism for used information in Sema. I feel it is too complex. So I feel maybe we can implement it in Serializer, not in Sema. The idea is, if we write anythiing internal to the current TU to the BMI, we can issue a diagnostic. But it requires we make the reduced BMI (https://clang.llvm.org/docs/StandardCPlusPlusModules.html#reduced-bmi) the default. Since the current full BMI will contain the full information after all. And according to my plan, the reduced BMI will be the default in clang21. So this feature is expected to be implemented that time.

@ChuanqiXu9 ChuanqiXu9 added the clang:modules C++20 modules and Clang Header Modules label Oct 15, 2024
@llvmbot
Copy link
Member

llvmbot commented Oct 15, 2024

@llvm/issue-subscribers-clang-modules

Author: Chuanqi Xu (ChuanqiXu9)

The wording is at http://eel.is/c++draft/basic.link#14

The simplest example is:

export module a;
static int a = 43;
export inline int func() {
    return a;
}

According to the spec, we should reject the example and say “the TU-local entity ‘a’ is an exposure”. But we didn’t implement this. We would accept the example silently. The practical affect this is, user may get weird diagnostic informations triggered by other places. Or, in the worst case, they may get runtime errors if the static variable have dyanmic initializers.

The reason why I didn’t implement this is, to implement this now, we may need to implement something more complex than the existing mechanism for used information in Sema. I feel it is too complex. So I feel maybe we can implement it in Serializer, not in Sema. The idea is, if we write anythiing internal to the current TU to the BMI, we can issue a diagnostic. But it requires we make the reduced BMI (https://clang.llvm.org/docs/StandardCPlusPlusModules.html#reduced-bmi) the default. Since the current full BMI will contain the full information after all. And according to my plan, the reduced BMI will be the default in clang21. So this feature is expected to be implemented that time.

ChuanqiXu9 added a commit to ChuanqiXu9/llvm-project that referenced this issue Jan 15, 2025
…ther TU

Close llvm#61427

And this is also helpful to implement
llvm#112294 partially.

The implementation strategy mimics
llvm#122887. This patch split the
internal declarations from the general lookup table so that other TU
can't find the internal declarations.
ChuanqiXu9 added a commit to ChuanqiXu9/llvm-project that referenced this issue Jan 17, 2025
…ther TU

Close llvm#61427

And this is also helpful to implement
llvm#112294 partially.

The implementation strategy mimics
llvm#122887. This patch split the
internal declarations from the general lookup table so that other TU
can't find the internal declarations.
ChuanqiXu9 added a commit that referenced this issue Jan 17, 2025
…ther TU (#123059)

Close #61427

And this is also helpful to implement
#112294 partially.

The implementation strategy mimics
#122887. This patch split the
internal declarations from the general lookup table so that other TU
can't find the internal declarations.
github-actions bot pushed a commit to arm/arm-toolchain that referenced this issue Jan 17, 2025
… found by other TU (#123059)

Close llvm/llvm-project#61427

And this is also helpful to implement
llvm/llvm-project#112294 partially.

The implementation strategy mimics
llvm/llvm-project#122887. This patch split the
internal declarations from the general lookup table so that other TU
can't find the internal declarations.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:modules C++20 modules and Clang Header Modules
Projects
None yet
Development

No branches or pull requests

2 participants