swiftlang / swift

The Swift Programming Language
https://swift.org
Apache License 2.0
67.53k stars 10.35k forks source link

[SR-9478] WriteableKeyPath with IUO crash #51940

Open mbrandonw opened 5 years ago

mbrandonw commented 5 years ago
Previous ID SR-9478
Radar None
Original Reporter @mbrandonw
Type Bug
Environment Xcode 10.1 (10B61) Swift 4.2
Additional Detail from JIRA | | | |------------------|-----------------| |Votes | 0 | |Component/s | Compiler | |Labels | Bug | |Assignee | None | |Priority | Medium | md5: 615ff1c11387ed7d548c7b719f700cc6

Issue Description:

The following code crashes:

func set<Root, Value>(
  _ keyPath: WritableKeyPath<Root, Value>,
  _ value: Value
  )
  -> (Root) -> Root {

    return { root in
      var copy = root
      copy[keyPath: keyPath] = value
      return copy
    }
}

var formatter = NumberFormatter()
formatter.locale = Locale(identifier: "")
let code = "USD"
set(\.currencyCode, code)(formatter)

with the stack trace:

 Fatal error: Unexpectedly found nil while unwrapping an Optional value
 Playground execution failed:

error: Execution was interrupted, reason: EXC_BAD_INSTRUCTION (code=EXC_I386_INVOP, subcode=0x0).
 The process has been left at the point where it was interrupted, use "thread return -x" to return to the state before expression evaluation.
 * thread #&#8203;1, queue = 'com.apple.main-thread', stop reason = EXC_BAD_INSTRUCTION (code=EXC_I386_INVOP, subcode=0x0)
 * frame #&#8203;0: 0x0000000104b9d941 libswiftCore.dylib`function signature specialization <Arg[2] = Dead, Arg[3] = Dead> of Swift._fatalErrorMessage(_: Swift.StaticString, _: Swift.StaticString, file: Swift.StaticString, line: Swift.UInt, flags: Swift.UInt32) -> Swift.Never + 113
 frame #&#8203;1: 0x0000000104c14fca libswiftCore.dylib`function signature specialization <Arg[1] = Dead, Arg[2] = Dead, Arg[3] = Dead, Arg[5] = Exploded> of Swift.RawKeyPathComponent.projectMutableAddress<A, B>(_: Swift.UnsafeRawPointer, from: A.Type, to: B.Type, isRoot: Swift.Bool, keepAlive: inout Swift.Optional<Swift.AnyObject>) -> Swift.UnsafeRawPointer + 1210
 frame #&#8203;2: 0x0000000104ad5fd1 libswiftCore.dylib`formalMutation #&#8203;1 <A, B><A1>(A1) -> Swift.UnsafeMutablePointer<B> in closure #&#8203;1 (Swift.KeyPathBuffer) -> Swift.UnsafeMutablePointer<B> in Swift.ReferenceWritableKeyPath.projectMutableAddress(from: A) -> (pointer: Swift.UnsafeMutablePointer<B>, owner: Swift.Optional<Swift.AnyObject>) + 705
 frame #&#8203;3: 0x0000000104ad5b8d libswiftCore.dylib`closure #&#8203;1 (Swift.KeyPathBuffer) -> Swift.UnsafeMutablePointer<B> in Swift.ReferenceWritableKeyPath.projectMutableAddress(from: A) -> (pointer: Swift.UnsafeMutablePointer<B>, owner: Swift.Optional<Swift.AnyObject>) + 989
 frame #&#8203;4: 0x0000000104ad5662 libswiftCore.dylib`Swift.ReferenceWritableKeyPath.projectMutableAddress(from: Swift.UnsafePointer<A>) -> (pointer: Swift.UnsafeMutablePointer<B>, owner: Swift.Optional<Swift.AnyObject>) + 146
 frame #&#8203;5: 0x0000000104ad8153 libswiftCore.dylib`Swift._projectKeyPathWritable<A, B>(root: Swift.UnsafeMutablePointer<A>, keyPath: Swift.WritableKeyPath<A, B>) -> (Swift.UnsafeMutablePointer<B>, Swift.Optional<Swift.AnyObject>) + 19
 frame #&#8203;6: 0x00000001053d7a39 $__lldb_expr15`closure #&#8203;1 in set<A, B>(root=0x00007ffeef0aa420, keyPath=0x0000000100bff048, value="USD") at _.xcplaygroundpage:11
 frame #&#8203;7: 0x00000001053d7e38 $__lldb_expr15`partial apply for closure #&#8203;1 in set<A, B>(_:_:) at <compiler-generated>:0
 frame #&#8203;8: 0x00000001053d746c $__lldb_expr15`main at _.xcplaygroundpage:19
 frame #&#8203;9: 0x0000000100b571a0 com.apple.dt.Xcode.PlaygroundStub-macosx
 frame #&#8203;10: 0x00007fff2b346f8c CoreFoundation`__invoking___ + 140
 frame #&#8203;11: 0x00007fff2b346e5f CoreFoundation`-[NSInvocation invoke] + 311
 frame #&#8203;12: 0x00007fff2b3a853d CoreFoundation`-[NSInvocation invokeWithTarget:] + 56
 frame #&#8203;13: 0x00007fff54448bf5 ViewBridge`__68-[NSVB_ViewServiceImplicitAnimationDecodingProxy forwardInvocation:]_block_invoke_2 + 46
 frame #&#8203;14: 0x00007fff5440dec6 ViewBridge`-[NSViewServiceMarshal withHostWindowFrameAnimationInProgress:perform:] + 53
 frame #&#8203;15: 0x00007fff54448bbe ViewBridge`__68-[NSVB_ViewServiceImplicitAnimationDecodingProxy forwardInvocation:]_block_invoke + 113
 frame #&#8203;16: 0x00007fff28924731 AppKit`+[NSAnimationContext runAnimationGroup:] + 55
 frame #&#8203;17: 0x00007fff289246e7 AppKit`+[NSAnimationContext runAnimationGroup:completionHandler:] + 82
 frame #&#8203;18: 0x00007fff5446b154 ViewBridge`runAnimationGroup + 295
 frame #&#8203;19: 0x00007fff54448695 ViewBridge`+[NSVB_View _animateWithAttributes:animations:completion:] + 490
 frame #&#8203;20: 0x00007fff54448b28 ViewBridge`-[NSVB_ViewServiceImplicitAnimationDecodingProxy forwardInvocation:] + 206
 frame #&#8203;21: 0x00007fff2b39080e CoreFoundation`___forwarding___ + 780
 frame #&#8203;22: 0x00007fff2b390478 CoreFoundation`__forwarding_prep_0___ + 120
 frame #&#8203;23: 0x00007fff2b346f8c CoreFoundation`__invoking___ + 140
 frame #&#8203;24: 0x00007fff2b346e5f CoreFoundation`-[NSInvocation invoke] + 311
 frame #&#8203;25: 0x00007fff2b3a853d CoreFoundation`-[NSInvocation invokeWithTarget:] + 56
 frame #&#8203;26: 0x00007fff54410973 ViewBridge`-[NSVB_QueueingProxy forwardInvocation:] + 324
 frame #&#8203;27: 0x00007fff2b39080e CoreFoundation`___forwarding___ + 780
 frame #&#8203;28: 0x00007fff2b390478 CoreFoundation`__forwarding_prep_0___ + 120
 frame #&#8203;29: 0x00007fff2b346f8c CoreFoundation`__invoking___ + 140
 frame #&#8203;30: 0x00007fff2b346e5f CoreFoundation`-[NSInvocation invoke] + 311
 frame #&#8203;31: 0x00007fff2b3a853d CoreFoundation`-[NSInvocation invokeWithTarget:] + 56
 frame #&#8203;32: 0x00007fff2b39080e CoreFoundation`___forwarding___ + 780
 frame #&#8203;33: 0x00007fff2b390478 CoreFoundation`__forwarding_prep_0___ + 120
 frame #&#8203;34: 0x00007fff2b346f8c CoreFoundation`__invoking___ + 140
 frame #&#8203;35: 0x00007fff2b346e5f CoreFoundation`-[NSInvocation invoke] + 311
 frame #&#8203;36: 0x00007fff543dd6cd ViewBridge`__deferNSXPCInvocationOntoMainThread_block_invoke + 237
 frame #&#8203;37: 0x00007fff543d2af3 ViewBridge`__wrapBlockWithVoucher_block_invoke + 37
 frame #&#8203;38: 0x00007fff543d28a4 ViewBridge`__deferBlockOntoMainThread_block_invoke_2 + 553
 frame #&#8203;39: 0x00007fff2b374ca7 CoreFoundation`__CFRUNLOOP_IS_CALLING_OUT_TO_A_BLOCK__ + 12
 frame #&#8203;40: 0x00007fff2b337ced CoreFoundation`__CFRunLoopDoBlocks + 395
 frame #&#8203;41: 0x00007fff2b337a49 CoreFoundation`__CFRunLoopRun + 2834
 frame #&#8203;42: 0x00007fff2b336ce4 CoreFoundation`CFRunLoopRunSpecific + 463
 frame #&#8203;43: 0x00007fff2a5d0895 HIToolbox`RunCurrentEventLoopInMode + 293
 frame #&#8203;44: 0x00007fff2a5d05cb HIToolbox`ReceiveNextEventCommon + 618
 frame #&#8203;45: 0x00007fff2a5d0348 HIToolbox`_BlockUntilNextEventMatchingListInModeWithFilter + 64
 frame #&#8203;46: 0x00007fff2888d95b AppKit`_DPSNextEvent + 997
 frame #&#8203;47: 0x00007fff2888c6fa AppKit`-[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:] + 1362
 frame #&#8203;48: 0x00007fff543db226 ViewBridge`-[NSViewServiceApplication nextEventMatchingMask:untilDate:inMode:dequeue:] + 92
 frame #&#8203;49: 0x00007fff2888675d AppKit`-[NSApplication run] + 699
 frame #&#8203;50: 0x00007fff28875e97 AppKit`NSApplicationMain + 780
 frame #&#8203;51: 0x00007fff58607f1f libxpc.dylib`_xpc_objc_main + 612
 frame #&#8203;52: 0x00007fff586079e5 libxpc.dylib`xpc_main + 433
 frame #&#8203;53: 0x00007fff543cca76 ViewBridge`-[NSXPCSharedListener resume] + 16
 frame #&#8203;54: 0x00007fff543d5a03 ViewBridge`NSViewServiceApplicationMain + 2877
 frame #&#8203;55: 0x0000000100b571ca com.apple.dt.Xcode.PlaygroundStub-macosx`main + 42
 frame #&#8203;56: 0x00007fff583d1085 libdyld.dylib`start + 1
 frame #&#8203;57: 0x00007fff583d1085 libdyld.dylib`start + 1

Two different things will fix it:

 -set(\.currencyCode, code)(formatter)
 +set(\NumberFormatter.currencyCode, code)(formatter)

or

 -set(\.currencyCode, code)(formatter)
 +set(\.currencyCode, .some(code))(formatter)
mbrandonw commented 5 years ago

Hi @jckarter! We ran into this one today. It has a pretty simple repro.

jckarter commented 5 years ago

I wouldn't expect IUO forcing to "win" the type check here. cc @rudkx @xedin

xedin commented 5 years ago

@jckarter Looks like There is only one solution we find in this case which requires `currencyCode` to be force unwrapped, because `set` function type is - (WriableKeyPath\<T, U>, U)` and we attempt `String` first based on type of `code`. `String?` could be attempted in this case but only if `String` where to fail, but it doesn't because there is a IUO involved.

Unfortunately it doesn't seem like we could do much in this case from type-checker perspective, @rudkx Any ideas?

rudkx commented 5 years ago

It seems like this ought to work and the fact that it doesn't today is an artifact of some implementation choices.

I took a quick look yesterday and it appeared that we were only attempting `String` for the type, but it seems like it would be reasonable to defer binding the type for the type variable until after we've selected a type for `Root` and determined the type of `currencyCode`.

xedin commented 5 years ago

@rudkx Yes, it seems like there is an ordering problem for this example but I'm not sure what the sane rule for changing it might be:

  ($T1 involves_type_vars bindings={(supertypes of) NumberFormatter})
  ($T2 involves_type_vars bindings={(supertypes of) String})
  ($T6 involves_type_vars bindings={(subtypes of) WritableKeyPath<$T1, $T2>})
  Initial bindings: $T1 := NumberFormatter
  (attempting type variable $T1 := NumberFormatter
    ($T2 involves_type_vars bindings={(supertypes of) String})
    ($T6 involves_type_vars bindings={(subtypes of) WritableKeyPath<NumberFormatter, $T2>})
    Initial bindings: $T2 := String
    (attempting type variable $T2 := String
      ($T6 bindings={(subtypes of) WritableKeyPath<NumberFormatter, String>})
      Initial bindings: $T6 := WritableKeyPath<NumberFormatter, String>
      (attempting type variable $T6 := WritableKeyPath<NumberFormatter, String>
        (overload set choice binding $T4 := @lvalue String?)
        ($T4 involves_type_vars bindings={(subtypes of) String})
        (attempting disjunction choice $T4 bind @lvalue String? [[locator@0x7fbe391cad28 [KeyPath@<REPL Input>:1:5 -> key path component #&#8203;0 -> implicitly unwrapped disjunction choice]]];
          (failed constraint $T4 equal $T5 [[locator@0x7fbe391ca628 [KeyPath@<REPL Input>:1:5]]];)
        )
        (attempting disjunction choice $T4 bind @lvalue String [[locator@0x7fbe391cad28 [KeyPath@<REPL Input>:1:5 -> key path component #&#8203;0 -> implicitly unwrapped disjunction choice]]];
          (increasing score due to force of an implicitly unwrapped optional)
          (found solution 0 0 1 0 0 0 0 0 0 0 0)
        )
      )
    )
  )

In this case $T6 is worse than either $T1 and $T2. If we had a disjunction associated with $T4 earlier that would have helped, but is that possible?