Skip to content
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

SE-0072: Fully eliminate implicit bridging conversions from Swift #2419

Merged
merged 3 commits into from
May 7, 2016

Conversation

jopamer
Copy link
Contributor

@jopamer jopamer commented May 6, 2016

SE-0072: Fully eliminate implicit bridging conversions from Swift

Per Swift Evolution proposal SE-0072, these changes prevent the compiler from introducing implicit bridging conversions during type checking.

Per Swift Evolution proposal SE-0072, these changes prevent the compiler from introducing implicit bridging conversions during type checking.
@jopamer
Copy link
Contributor Author

jopamer commented May 6, 2016

@swift-ci Please test

self.z = SCNFloat(z)
self.x = SCNFloat(x as NSNumber)
self.y = SCNFloat(y as NSNumber)
self.z = SCNFloat(z as NSNumber)
Copy link
Contributor

@gribozavr gribozavr May 6, 2016

Choose a reason for hiding this comment

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

Oh, I don't think that was intended.

#if os(OSX)
public typealias SCNFloat = CGFloat
#elseif os(iOS) || os(tvOS)
public typealias SCNFloat = Float
#endif

Given this definition, this was intended as a plain conversion, not a bridge.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed - debugging in, though, we've definitely been introducing this conversion implicitly. I've filed SR-1432 to track my investigation of what's been going on.

@gribozavr
Copy link
Contributor

gribozavr commented May 6, 2016

I'm amazed how few of these we had in the testsuite.

@jopamer
Copy link
Contributor Author

jopamer commented May 6, 2016

@swift-ci Please test

1 similar comment
@jopamer
Copy link
Contributor Author

jopamer commented May 6, 2016

@swift-ci Please test

@jopamer
Copy link
Contributor Author

jopamer commented May 6, 2016

@swift-ci Please ASAN test

@tkremenek
Copy link
Member

@swift-ci smoke test OS X platform

@jopamer
Copy link
Contributor Author

jopamer commented May 7, 2016

@swift-ci smoke test OS X platform

@jopamer
Copy link
Contributor Author

jopamer commented May 7, 2016

@swift-ci Please test

@jopamer
Copy link
Contributor Author

jopamer commented May 7, 2016

The failing smoke tests should be fixed by @lattner's recent commits, so this is now safe to merge.

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.

4 participants