Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[SR-13364] keypath missing optional crashes compiler: "Inactive constraints left over?" #55805

Closed
mayoff opened this issue Aug 7, 2020 · 8 comments
Closed

[SR-13364] keypath missing optional crashes compiler: "Inactive constraints left over?" #55805

mayoff opened this issue Aug 7, 2020 · 8 comments
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. compiler The Swift compiler in itself crash Bug: A crash, i.e., an abnormal termination of software type checker Area → compiler: Semantic analysis

Comments

@mayoff
Copy link

mayoff commented Aug 7, 2020

Previous ID SR-13364
Radar rdar://problem/66706980
Original Reporter @mayoff
Type Bug
Status Resolved
Resolution Done
Additional Detail from JIRA
Votes 0
Component/s Compiler
Labels Bug, CompilerCrash, TypeChecker
Assignee None
Priority Medium

md5: 71e5c011ff187bd9e90ed4a902b8b2af

is duplicated by:

  • SR-13756 Abort: 6 when type checking an array expression that contains tuples of implicit enum instances.

Issue Description:

Here's the test case:

let keyPath: KeyPath<String?, Int?> = \.utf8.count

Here's the crash with Xcode 12 beta 4's toolchain:

Welcome to Apple Swift version 5.3 (swiftlang-1200.0.25.2 clang-1200.0.27.1).
Type :help for assistance.
  1> let keyPath: KeyPath<String?, Int?> = \.utf8.count
Score: 2 0 0 0 0 0 0 0 0 0 0 0 0
Contextual Type: KeyPath<String?, Int?> at [/var/folders/kn/1d839myx4tlghz34f_lh3hvc0000gn/T/repl1-8f3bab..swift:1:14 - line:1:35]
Type Variables:
  $T0 [noescape allowed] as String? @ locator@0x7fa1c9582a00 [KeyPath@/var/folders/kn/1d839myx4tlghz34f_lh3hvc0000gn/T/repl1-8f3bab..swift:1:39 -> key path root]
  $T1 [lvalue allowed] [noescape allowed] as String.UTF8View? @ locator@0x7fa1c9582aa8 [KeyPath@/var/folders/kn/1d839myx4tlghz34f_lh3hvc0000gn/T/repl1-8f3bab..swift:1:39 -> key path component #&#8203;0 -> key path component result]
  $T2 [lvalue allowed] [noescape allowed] as $T8? @ locator@0x7fa1c9582b90 [KeyPath@/var/folders/kn/1d839myx4tlghz34f_lh3hvc0000gn/T/repl1-8f3bab..swift:1:39 -> key path component #&#8203;1 -> key path component result]
  $T3 [noescape allowed] as Int? @ locator@0x7fa1c9582c60 [KeyPath@/var/folders/kn/1d839myx4tlghz34f_lh3hvc0000gn/T/repl1-8f3bab..swift:1:39 -> key path value]
  $T4 [noescape allowed] as KeyPath<String?, Int?> @ locator@0x7fa1c9582d20 [KeyPath@/var/folders/kn/1d839myx4tlghz34f_lh3hvc0000gn/T/repl1-8f3bab..swift:1:39 -> key path type]
  $T6 [lvalue allowed] [noescape allowed] as String.UTF8View @ locator@0x7fa1c9582a88 [KeyPath@/var/folders/kn/1d839myx4tlghz34f_lh3hvc0000gn/T/repl1-8f3bab..swift:1:39 -> key path component #&#8203;0]
  $T8 [lvalue allowed] [noescape allowed] as Int @ locator@0x7fa1c9582b70 [KeyPath@/var/folders/kn/1d839myx4tlghz34f_lh3hvc0000gn/T/repl1-8f3bab..swift:1:39 -> key path component #&#8203;1]


Active Constraints:


Inactive Constraints:
  $T4 key path from $T0 -> $T3 [[locator@0x7fa1c9582a20 [KeyPath@/var/folders/kn/1d839myx4tlghz34f_lh3hvc0000gn/T/repl1-8f3bab..swift:1:39]]];
Resolved overloads:
  selected overload set choice String.utf8: $T6 == String.UTF8View
  selected overload set choice String.UTF8View.count: $T8 == Int




Fixes:
  [fix: unwrap optional base of member lookup] @ locator@0x7fa1c9582a88 [KeyPath@/var/folders/kn/1d839myx4tlghz34f_lh3hvc0000gn/T/repl1-8f3bab..swift:1:39 -> key path component #&#8203;0]
  [fix: unwrap optional base of member lookup] @ locator@0x7fa1c9582b70 [KeyPath@/var/folders/kn/1d839myx4tlghz34f_lh3hvc0000gn/T/repl1-8f3bab..swift:1:39 -> key path component #&#8203;1]
LLVM ERROR: Inactive constraints left over?
@mayoff
Copy link
Author

mayoff commented Aug 7, 2020

The corrected keypath literal does not crash the compiler:

let keyPath: KeyPath<String?, Int?> = \.?.utf8.count

@LucianoPAlmeida
Copy link
Collaborator

cc @xedin @hamishknight

@LucianoPAlmeida
Copy link
Collaborator

Just reproduced on a near master

@hamishknight
Copy link
Collaborator

@swift-ci create

@LucianoPAlmeida
Copy link
Collaborator

@xedin @hamishknight @hborla I was taking a look at that and the problem seems to be related to the fact the value is optional `Int` so the solver is not able to find a binding for that and the result is a system with an unresolved KeyPath constraint, therefore, a crash. I kinda fix by marking the keypath type var fully bound, allowing the solver to wait until root and value(in this case rvalueBase) so we are able to find the optional binding for the key path value and resolve key path constraint. But that seems to cause a regression in a specific test-case

swift/test/Constraints/rdar62201037.swift

Line 28 in 782863c

rt |> map(\.str ^^^ s)
which I'm not sure is completely incorrect, but is indeed a change that broke a code that compiled so probably not the ideal way to go.

So my question would be if you think that is indeed the problem and we are at least in the right direction, or am I missing something?
I can open a draft PR and we can discuss from there which would be a good solution for this issue 🙂

@hamishknight
Copy link
Collaborator

@LucianoPAlmeida Sorry for the delayed response! If I recall correctly, the problem here is that an overload type variable is being bound (from the context) before the overload is being resolved, leaving the key path constraint unsolved. This is because key path constraints currently rely on overloads being resolved when they get bound (as this triggers the re-activation of the constraint, and the key path needs to dig out the overload choice).

The proper solution to this would be to re-design how we solve key-path constraints. Ideally we would have it so the key path component validation and concrete key path type inference logic only gets run when the last overload in the chain is resolved, and we would have a proper way of propagating the root type of the path from the context without relying on obtaining it from the constraint system's contextual type (which would solve e.g SR-8383).

I was going to have a crack at this, but never got round to it – I'd be happy to answer any questions if you want to take it on! I believe @xedin has also had some thoughts on how to re-do key path solving and may be able to help.

@LucianoPAlmeida
Copy link
Collaborator

@hamishknight No worries, thanks for the explanation.
We actually have some discussions about that in SR-12390 and in #33206 And also, in this PR @xedin actually described the idea here: #30764 (comment) is that we do a similar approach used by resolveClosure. I've actually started to take a look at it a while ago but was not able to do much so feel free to take on 🙂

@xedin
Copy link
Member

xedin commented Mar 15, 2021

Fixed by #36427 Please use next available main branch snapshot to verify.

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
@AnthonyLatsis AnthonyLatsis added the crash Bug: A crash, i.e., an abnormal termination of software label Dec 12, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. compiler The Swift compiler in itself crash Bug: A crash, i.e., an abnormal termination of software type checker Area → compiler: Semantic analysis
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@xedin @mayoff @LucianoPAlmeida @hamishknight @AnthonyLatsis