-
Notifications
You must be signed in to change notification settings - Fork 10.4k
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
Rebase: [ConstraintSystem] Ability to treat key path literal syntax as function type (Root) -> Value. #23435
Conversation
@swift-ci please smoke test |
@swift-ci please build toolchain |
@swift-ci please test source compatibility |
Linux Toolchain (Ubuntu 16.04) Install command |
macOS Toolchain Install command |
324d616
to
bba37e7
Compare
@swift-ci please test compiler performance |
1 similar comment
@swift-ci please test compiler performance |
@swift-ci please test source compatibility |
Going to rerun the performance test to see if the small regression holds up. |
@swift-ci please test compiler performance |
Summary for master fullUnexpected test results, excluded stats for CoreStore, Tagged, ProcedureKit, Wordy, Deferred Regressions found (see below) Debug-batchdebug-batch briefRegressed (0)
Improved (0)
Unchanged (delta < 1.0% or delta < 100.0ms) (3)
debug-batch detailedRegressed (2)
Improved (0)
Unchanged (delta < 1.0% or delta < 100.0ms) (97)
Releaserelease briefRegressed (0)
Improved (0)
Unchanged (delta < 1.0% or delta < 100.0ms) (3)
release detailedRegressed (0)
Improved (0)
Unchanged (delta < 1.0% or delta < 100.0ms) (23)
|
I just tried the toolchain.
|
@damuellen Can you try the latest master snapshot and see if you have the same issue? I'm not sure how this PR would affect an expression with no key paths. |
@swift-ci Please build toolchain macOS platform |
bba37e7
to
c36d348
Compare
@swift-ci please test |
@swift-ci please test source compatibility |
Build failed |
Build failed |
@swift-ci please test macos platform |
Build failed |
Fix autoclosure param interface type when it involves archetypes. Had some repeated locals from moving this block of code around - cleaned up. Alternate implementation where KeyPathExpr is essentially a literal type and can be either a KeyPath(R,V) or (R)->V. Some unneccessary code now. Implicit closure pieces need valid sourceLocs in case a coerce expr needs to refer to the beginning of the closure in `\.foo as (A) -> B`. Removed explicit noescape from function type so `let a: (A) -> B = \.foo` is valid. Remove optimization that optional path is always read-only KP type in CSGen, since it can also now be of function type.
Necessary to keep SE-0249 from failing tests.
335e774
to
223fde6
Compare
Will this PR and SE-0249 be merged within the Swift 5.1 release? |
@pitiphong-p It's not looking like it'll be in 5.1, unfortunately—this implementation works in most cases, but making it work in all cases (particularly with optional chaining key paths) requires some refactoring to fix longstanding key path bugs in the typechecker, and then that refactoring has to be backported to 5.1, which is a bit risky. The proposal is still approved and the basic approach this PR takes is still valid. |
@brentdax Thank you for the explanation. |
Rebase of #19448 with an important bug fix. The original PR did not have a rule that would cause the solver to prefer
KeyPath
solutions to function solutions, making them ambiguous.