Skip to content

SILGen: Skip the value-to-id peephole for types with nontrivial SIL lowering. #5590

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
Nov 2, 2016

Conversation

jckarter
Copy link
Contributor

@jckarter jckarter commented Nov 2, 2016

The emitNativeToBridgedValue code is not well-factored to pass down the formal type of the value being bridged, which is necessary to build a generic call to _bridgeAnythingToObjectiveC in the fallback case. To work around this issue, disable the expression peephole when the type is not a valid lowered SIL type as is. Short-term fix for rdar://problem/28318984.

…owering.

The `emitNativeToBridgedValue` code is not well-factored to pass down the formal type of the value being bridged, which is necessary to build a generic call to _bridgeAnythingToObjectiveC in the fallback case. To work around this issue, disable the expression peephole when the type is not a valid lowered SIL type as is. Short-term fix for rdar://problem/28318984.
@jckarter
Copy link
Contributor Author

jckarter commented Nov 2, 2016

@swift-ci Please smoke test

@jckarter
Copy link
Contributor Author

jckarter commented Nov 2, 2016

@slavapestov How does this look for 3.0?

@jckarter jckarter merged commit b2562da into swiftlang:master Nov 2, 2016
@slavapestov
Copy link
Contributor

Looks good. I ran into the issue where the formal type is not available when originally implementing the part of the id-as-Any stuff that was on my plate... I don't remember what the difficulty was but it sounds like a good issue to fix at some point.

@jckarter
Copy link
Contributor Author

jckarter commented Nov 2, 2016

Yeah, we ought to refactor the bridging code to plumb the formal type through. That's too big a project to take for 3.0 at this point.

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