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.

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

[SR-14096] KeyPath when used as a multi-argument function type #56482

Closed
swift-ci opened this issue Jan 24, 2021 · 13 comments
Closed

[SR-14096] KeyPath when used as a multi-argument function type #56482

swift-ci opened this issue Jan 24, 2021 · 13 comments
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. compiler The Swift compiler itself

Comments

@swift-ci
Copy link
Contributor

Previous ID SR-14096
Radar rdar://problem/73600325
Original Reporter artdivin (JIRA User)
Type Bug
Status Resolved
Resolution Done

Attachment: Download

Environment

Swift 5.3.2 (swiftlang-1200.0.45 clang-1200.0.32.28)

Xcode 12.4 (12D4e)

Additional Detail from JIRA
Votes 0
Component/s Compiler
Labels Bug
Assignee artdivin (JIRA)
Priority Medium

md5: 23293c02146f4ec64ff505bd8d184b25

Issue Description:

Hello!

I have found an undefined behaviour when compiler doesn't generate correct error messages, i.e. compilation succeeds if the type is omitted, but when the type is specified the compilation error is generated correctly.

See cases #2 and #3 in the attached screenshot.

I will add textual representation into the description as well:

import Foundation


extension String {
    
    var filterOut : (Self) throws -> Bool {
        { $0.contains("a") }
    }
    
    var myLocalizedCompared : (Self, Self) throws -> Bool {
        {
            $0.localizedStandardCompare($1) == .orderedAscending
        }
    }
    
}


var unsorted : [String] = [ "asd", "bcd", "def" ]


// 1. compiles, provides output "asd"
print(unsorted.filter { $0.contains("a") })


// 2. compiles, provides empty output
print(unsorted.filter(\.filterOut))


// 3. error: Cannot convert value of type 'KeyPath<String, (String) throws -> Bool>'
// to expected argument type '(String) throws -> Bool'
//print(unsorted.filter(\String.filterOut))


// 4. compiles: however, filter closure signature: (T) throws -> Bool
// and we are passing (T, T) throws -> Bool
// output is empty
print(unsorted.filter(\.myLocalizedCompared))


// 5. error: Cannot convert key path into
// a multi-argument function type '(String, String) throws -> Bool'
//unsorted.sort(by: \.myLocalizedCompared)
@typesanitizer
Copy link

@swift-ci create

@xedin
Copy link
Contributor

xedin commented Jan 28, 2021

luciano (JIRA User) You might be interested to take a look at #3 here, #4 I'd say is unfortunately not-to-be-fixed without risking source compatibility impact since this is just a tuple splat behavior, curtesy of pre SE-0110 Swift, where `T` would be bound to `(T, T)` passed as an argument.

@LucianoPAlmeida
Copy link
Contributor

Sure! Thanks for the ping @xedin =]

@LucianoPAlmeida
Copy link
Contributor

For what I understand #3 should definitely fail, so it would be just the case of providing a better diagnostic. Still didn't look but #2 seems a bit curious in what type is getting inferred for Root in order for this to compile ... I'll take a look

@LucianoPAlmeida
Copy link
Contributor

Yeah, in an assert build #2 crashes on assertion
Assertion failed: (!componentTy || componentTy->hasUnresolvedType() || componentTy->getWithoutSpecifierType()->isEqual(leafTy)), function visitKeyPathExpr, file <path>/swift/lib/Sema/CSApply.cpp, line 4765.

@LucianoPAlmeida
Copy link
Contributor

The problem for the #2 case is that type checker don't detect that for the of key path value (Self) throws -> Bool doesn't match the expected contextual function result type Bool.
Inference for key paths still have some issues that we discussed some time ago in SR-12390, but I'm not sure if this have something to do with this but could possible be related.
As far as my understanding #2 should produce the same diagnostic as #3 (or maybe a improvement on the wording making more clear that user is trying to use key path as function but the inferred key path value don't match expected function result type)

@xedin
Copy link
Contributor

xedin commented Feb 11, 2021

@LucianoPAlmeida I think #2 and #3 should type-check because they are equivalent to:

var unsorted : [String] = [ "asd", "bcd", "def" ]

var fn: (String) -> Bool = { $0.contains("a") }
print(unsorted.filter(fn))

It seems to be the reason why they don't is because `Self` is misinterpreted.

@LucianoPAlmeida
Copy link
Contributor

Ah that makes sense, but the same behavior also happens if `filterOut` is declared as explicit as`String`

var filterOut : (String) throws -> Bool { { $0.contains("a") } }

My reasoning in thinking that it could be correct diagnose #2 as a mismatch the same way #3, was that based on key path as functions, #3
should be equivalent to

`_ = unsorted.filter({ $0[keyPath: \String.filterOut] })` 

where the result of key path application is a unapplied function type `(String) -> Bool` that indeed doesn't match the expected return type `Bool`

@xedin
Copy link
Contributor

xedin commented Feb 11, 2021

Ah yes, looks like I misunderstand what SE-0249 was all about - it's supposed to be possible to convert keypath to a function that accepts a base and returns a value of member type referenced by a key path, by that logic `.filterOut` is converted into `(String) -> (Self) throws -> Bool` when used as `filter(.filterOut)`. I agree that both #2 and #3 should produce identical errors.

@LucianoPAlmeida
Copy link
Contributor

Awesome, I still didn't look to deep into it but I'm wondering if the cause for the solver can't detect the mismatch for #2 has something related to what we discussed some time ago in SR-12390 ... I'll take a look soon =]

@LucianoPAlmeida
Copy link
Contributor

Hey @xedin, sorry this is in progress for a while but in fact I haven't be able to really work on it, been in a bit of struggle with other things... but will try to pick this up again this week =]

@xedin
Copy link
Contributor

xedin commented Mar 17, 2021

@LucianoPAlmeida No worries! Take your time.

@xedin
Copy link
Contributor

xedin commented Mar 22, 2021

Fixed by #36531 artdivin (JIRA User) Please use next available main branch snapshot to verify and close.

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A deviation from expected or documented behavior. Also: expected but undesirable behavior. compiler The Swift compiler itself
Projects
None yet
Development

No branches or pull requests

4 participants