Skip to content

SILGen: Fix assertion failure when emitting foreign-to-native thunk for a method with inout 'self' #34456

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

Conversation

slavapestov
Copy link
Contributor

We import a non-const pointer 'self' type as a mutating method.

The getParameterTypes() helper method in SILGenBridging.cpp would
assert upon encountering the inout 'self' parameter, even though
emitForeignToNativeThunk() would still emit correct code as long
as the 'self' type was not bridged.

Relax the assertion a bit to hopefully still catch bugs where
other parameters are unexpectedly 'inout', but allow it on 'self',
and add a test.

Fixes rdar://problem/70346482.

@slavapestov slavapestov requested a review from jckarter October 27, 2020 04:35
@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test

@slavapestov slavapestov force-pushed the foreign-to-native-inout-self branch from d236cb2 to aa45221 Compare October 27, 2020 16:59
@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test

Copy link
Contributor

@jckarter jckarter left a comment

Choose a reason for hiding this comment

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

Thanks for cleaning this up, Slava!

…or a method with inout 'self'

We import a C function taking a non-const pointer 'self' parameter
as a mutating method.

The getParameterTypes() helper method in SILGenBridging.cpp would
assert upon encountering the inout 'self' parameter, even though
emitForeignToNativeThunk() would still emit correct code as long
as the 'self' type was not bridged.

Relax the assertion a bit to hopefully still catch bugs where
other parameters are unexpectedly 'inout', but allow it on 'self',
and add a test.

Fixes <rdar://problem/70346482>.
@slavapestov slavapestov force-pushed the foreign-to-native-inout-self branch from aa45221 to 51c1fa9 Compare October 27, 2020 17:46
@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test

@slavapestov
Copy link
Contributor Author

@swift-ci Please smoke test Linux

@slavapestov slavapestov merged commit 5910d82 into swiftlang:main Oct 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants