Skip to content

Conversation

Xazax-hun
Copy link
Contributor

This triggers a crash. Unfortunately, adding support is not that straightforward so skipping these accessors for now.

rdar://134425834

This triggers a crash. Unfortunately, adding support is not that
straightforward so skipping these accessors for now.

rdar://134425834
@Xazax-hun Xazax-hun added the c++ interop Feature: Interoperability with C++ label Aug 30, 2024
@Xazax-hun
Copy link
Contributor Author

@swift-ci please smoke test

}
}
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please also add a test where a Swift subscript has both get and _read accessors? I think we would want the get accessor to be used in that case, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried adding both and got this error:

error: subscript cannot provide both a 'read' accessor and a getter

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah alright, I thought that is allowed, my bad

@Xazax-hun Xazax-hun merged commit 25a8885 into main Aug 30, 2024
3 checks passed
@Xazax-hun Xazax-hun deleted the gaborh/read-accessor branch August 30, 2024 21:27
Xazax-hun added a commit that referenced this pull request Sep 2, 2024
The reverse interop code assumed that there is always a get accessor for
subscript which is not always the case. This PR prevents the crash by
not exporting subscript when get accessor is not availableA.
Scope: C++ reverse interop.
Risk: Low, we just skip problematic operators.
Testing: Regression test added.
Issue: rdar://134425834
Reviewer: @egorzhdan
Original PR: #76170
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