Skip to content

Sema: Check availability of key path components [4.2] #17780

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

Conversation

slavapestov
Copy link
Contributor

@slavapestov slavapestov commented Jul 6, 2018

  • Description: We weren't checking availability for keypath components. This could result in runtime crashes if you reference selectors that don't exist yet, or linker errors if you reference private things from an inlinable function body.

  • Origination: This was an issue ever since keypaths were introduced in Swift 4.0.

  • Tested: New tests added.

  • Risk: Pretty low, because the availability checking code itself is well-exercised. The main risk is source compatibility issues, but impacted code was not completely valid in the first place anyway.

  • Bugs: Fixes https://bugs.swift.org/browse/SR-7406 / rdar://problem/39319212.

  • Reviewed by: @jckarter

@slavapestov
Copy link
Contributor Author

@swift-ci Please test

@swift-ci
Copy link
Contributor

swift-ci commented Jul 6, 2018

Build failed
Swift Test OS X Platform
Git Sha - df9258b

@slavapestov
Copy link
Contributor Author

@shahmishal is this a known issue?

/Users/buildnode/jenkins/workspace/swift-PR-osx/branch-swift-4.2-branch/swift/test/stdlib/os_log_format.m:13:11: error: expected string not found in input
// CHECK: PASS
          ^
<stdin>:1:1: note: scanning from here
FAIL: during: "simple long double %Lf": command 0 data mismatch

@shahmishal
Copy link
Member

@slavapestov I have not seen this failure before.

@slavapestov
Copy link
Contributor Author

@swift-ci Please test macOS

@swift-ci
Copy link
Contributor

swift-ci commented Jul 6, 2018

Build failed
Swift Test OS X Platform
Git Sha - df9258b

@slavapestov
Copy link
Contributor Author

@shahmishal its non deterministic...

@shahmishal
Copy link
Member

@slavapestov going to disable this test. (41380265)

@shahmishal
Copy link
Member

@swift-ci test macOS

We would crash in these cases:

- If all lookup results were subscripts
- If all results were unavailable
@slavapestov
Copy link
Contributor Author

@swift-ci Please test

1 similar comment
@slavapestov
Copy link
Contributor Author

@swift-ci Please 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.

Looks good, thanks Slava!

@slavapestov slavapestov merged commit 7ee89cd into swiftlang:swift-4.2-branch Jul 6, 2018
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