Crash during optional key path access

[stephencelis]

Description

I have code that appends key paths that is crashing at runtime when passing the key path to a value's key path subscript.

Reproduction

struct Foo {
  var bar: Bar?
}
struct Bar {
  var baz: Int?
}
extension Optional {
  subscript(default default: Wrapped) -> Wrapped {
    self ?? `default`
  }
}

let foo = Foo()
let kp1 = \Foo.bar
let kp2 = kp1.appending(path: \.?.baz)
let kp3 = kp2.appending(path: \.[default: 5])
foo[keyPath: kp3]  // 💥

💥 Could not cast value of type 'Swift.Optional<()>' (0x1df122378) to 'Swift.Int' (0x1df11e138).

Stack dump

N/A

Expected behavior

I expect this code not to crash, and ideally return the default 5.

Environment

swift-driver version: 1.87.3 Apple Swift version 5.9.2 (swiftlang-5.9.2.2.56 clang-1500.1.0.2.5)
Target: arm64-apple-macosx14.0

Additional information

No response

[stephencelis]

/cc @Azoy, @xedin: I think you've both been in key path land recently.

[xedin]

This must be something in implementation of KeyPath itself, foo[keyPath: kp3] seems correct to me, it takes KeyPath<Foo, Int> and produces Int. Not sure whether Optional<()> even comes from since it not there in any of the solutions.

[jamieQ]

i've looked into the cause of this behavior a bit (associated swift forums thread here), and have some thoughts & questions on the issue, and what can potentially be done about it.

what i believe is happening is that the logic for keypath projection contains a built-in assumption that the presence of an optionally-chained component implies that the keypath must end in an optional value. this is explicitly enforced by an internal invariant (only enabled in internal builds), and the runtime cast failure leading to the crash is due to this nearby force-cast which assumes the invariant held.

now, brainstorming ideas for how to fix this – it seems to address the issue we will likely need a change to how optional-chains which result in nil 'break' out of the projection loop. currently they 'jump' to the end, but they instead probably should only advance to the next component that was explicitly appended. i think this information could probably be stored in the component header when an append operation is performed.

does that seem like a reasonable approach to take?

[stephencelis]

@jamieQ Thanks for looking into this! @Azoy may be able to steer you in the right direction, as he's working on #70451 and briefly looked at this issue.