Skip to content

[cxx-interop] Do not expose enums with optional generic cases #74966

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

Merged
merged 1 commit into from
Jul 4, 2024

Conversation

Xazax-hun
Copy link
Contributor

Unfortunately, we cannot generate the C++ code for them just yet. There will be a follow-up PR to actually fix the underlying issue and expose such enums correctly.

rdar://129250756

@Xazax-hun Xazax-hun added the c++ interop Feature: Interoperability with C++ label Jul 4, 2024
@Xazax-hun
Copy link
Contributor Author

@swift-ci please smoke test

Copy link
Contributor

@egorzhdan egorzhdan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this issue specific to optionals, or would we fail to generate the C++ representation of any enum case that has an associated value with a generic parameter?

@Xazax-hun
Copy link
Contributor Author

or would we fail to generate the C++ representation of any enum case that has an associated value with a generic parameter

Yeah, currently the code path would try to dereference a null pointer for code like that. I tried to make a fix but it was more involved than I anticipated, so I decided to make a quick fix for now and will add support for this case in a follow-up PR.

Unfortunately, we cannot generate the C++ code for them just yet. There
will be a follow-up PR to actually fix the underlying issue and expose
such enums correctly.

rdar://129250756
@Xazax-hun Xazax-hun force-pushed the gaborh/stop-exporting-optional-generics branch from d7c6a40 to c56cca1 Compare July 4, 2024 14:42
@Xazax-hun
Copy link
Contributor Author

Rebased to resolve conflict.

@swift-ci please smoke test

Copy link
Contributor

@egorzhdan egorzhdan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, LGTM

@Xazax-hun Xazax-hun enabled auto-merge July 4, 2024 16:27
@Xazax-hun Xazax-hun merged commit 6cc6f7c into main Jul 4, 2024
3 checks passed
@Xazax-hun Xazax-hun deleted the gaborh/stop-exporting-optional-generics branch July 4, 2024 17:47
Xazax-hun added a commit that referenced this pull request Jul 5, 2024
Explanation:
The code path trying to generate the C++ header for enums that have
cases with associated values of a generic optional type resulted in null
pointer dereference. The proper fix is a bit more involved so this PR
just temporarily avoids exposing the problematic enums to C++.
Scope: C++ Reverse Interop
Risk: Low, we stop exposing enums that were crashing the compiler before.
Testing: Added a regression test.
Issue: rdar://129250756
Reviewer: @egorzhdan
Original PR: #74966
Xazax-hun added a commit that referenced this pull request Jul 5, 2024
Explanation:
The code path trying to generate the C++ header for enums that have
cases with associated values of a generic optional type resulted in null
pointer dereference. The proper fix is a bit more involved so this PR
just temporarily avoids exposing the problematic enums to C++.
Scope: C++ Reverse Interop
Risk: Low, we stop exposing enums that were crashing the compiler before.
Testing: Added a regression test.
Issue: rdar://129250756
Reviewer: @egorzhdan
Original PR: #74966
Xazax-hun added a commit that referenced this pull request Jul 8, 2024
Explanation:
The code path trying to generate the C++ header for enums that have
cases with associated values of a generic optional type resulted in null
pointer dereference. The proper fix is a bit more involved so this PR
just temporarily avoids exposing the problematic enums to C++.
Scope: C++ Reverse Interop
Risk: Low, we stop exposing enums that were crashing the compiler before.
Testing: Added a regression test.
Issue: rdar://129250756
Reviewer: @egorzhdan
Original PR: #74966
Xazax-hun added a commit that referenced this pull request Jul 8, 2024
Explanation:
The code path trying to generate the C++ header for enums that have
cases with associated values of a generic optional type resulted in null
pointer dereference. The proper fix is a bit more involved so this PR
just temporarily avoids exposing the problematic enums to C++.
Scope: C++ Reverse Interop
Risk: Low, we stop exposing enums that were crashing the compiler before.
Testing: Added a regression test.
Issue: rdar://129250756
Reviewer: @egorzhdan
Original PR: #74966
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ interop Feature: Interoperability with C++
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants