Skip to content

Fix StringTransform usage of forced casts. #2143

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
Apr 22, 2019

Conversation

drodriguez
Copy link
Contributor

In Linux, a (constant?) CFString cannot be force casted to String
directly. The code generated for those values with optimizations enabled
was simply a crash. The changes uses _swiftObject to correctly
transform to a Swift.String.

There some other cases in Foundation where other enums are initialized
from CF values, but in those cases the literals of the CF values are
used, which avoids this problem.

Should fix the problems introduced in #2087 in Linux.

PD: I still think that #2087 changes the meaning of the test. It should be reverted or removed.

@millenomi
Copy link
Contributor

… Oh, yep, that makes sense.

I have disabled a test that likely tripped on this issue in https://github.com/apple/swift-corelibs-foundation/pull/2142/files — can you revert the testExpectedToFail line about test_substringFromCFString and see if this fixes that?

Copy link
Contributor

@millenomi millenomi left a comment

Choose a reason for hiding this comment

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

Please test as above?

In Linux, a (constant?) CFString cannot be force casted to String
directly. The code generated for those values with optimizations enabled
was simply a crash. The changes uses `_swiftObject` to correctly
transform to a `Swift.String`.

There some other cases in Foundation where other enums are initialized
from CF values, but in those cases the literals of the CF values are
used, which avoids this problem.
@drodriguez drodriguez force-pushed the fix-string-transform-enum branch from 784a248 to b48e6b9 Compare April 18, 2019 18:53
@drodriguez
Copy link
Contributor Author

I re-enabled the test (I didn’t pull from last night, so in my working directory it wasn’t disabled). Should be ready to test.

@millenomi
Copy link
Contributor

@swift-ci please test

@drodriguez
Copy link
Contributor Author

I think I pushed the new version before the test finished, and CI didn’t start a new test run.

@millenomi
Copy link
Contributor

@swift-ci please test

@millenomi millenomi merged commit 0ad5b17 into swiftlang:master Apr 22, 2019
@drodriguez drodriguez deleted the fix-string-transform-enum branch July 8, 2019 21:07
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