Skip to content

[TypeChecker] Improve contextual mismatch diagnostics for key path #24308

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

Merged
merged 1 commit into from
Apr 26, 2019

Conversation

xedin
Copy link
Contributor

@xedin xedin commented Apr 26, 2019

Detect situations where key path doesn't have capability required
by a context e.g. read-only vs. writable, or either root or value
types are incorrect e.g.

struct S { let foo: Int }
let _: WritableKeyPath<S, Int> = \.foo

Here context requires a writable key path but foo property is
read-only.

@xedin
Copy link
Contributor Author

xedin commented Apr 26, 2019

@swift-ci please test

Detect situations where key path doesn't have capability required
by the context e.g. read-only vs. writable, or either root or value
types are incorrect e.g.

```swift
struct S { let foo: Int }
let _: WritableKeyPath<S, Int> = \.foo
```

Here context requires a writable key path but `foo` property is
read-only.
@xedin xedin force-pushed the improve-contextual-diags-for-keypath branch from 2fa85d1 to 94977ee Compare April 26, 2019 17:59
@xedin
Copy link
Contributor Author

xedin commented Apr 26, 2019

/cc @brentdax

@xedin
Copy link
Contributor Author

xedin commented Apr 26, 2019

@swift-ci please test

@swiftlang swiftlang deleted a comment from swift-ci Apr 26, 2019
@swiftlang swiftlang deleted a comment from swift-ci Apr 26, 2019
@swiftlang swiftlang deleted a comment from swift-ci Apr 26, 2019
Copy link
Contributor

@beccadax beccadax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So glad this works!

let _: ReferenceWritableKeyPath<A, Prop> = \.property
//expected-error@-1 {{cannot convert value of type 'WritableKeyPath<A, Prop>' to specified type 'ReferenceWritableKeyPath<A, Prop>'}}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Most of this test file puts the assertions on the same line or the line before; I think putting these on the line after might confuse future maintainers.

(This applies to the other changes in this file, too.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's going to be a big problem especially considering that most of the tests (if not all) have expected checks on the same or line after the one checked. These TODOs where more of an exception to the rule.

@xedin xedin merged commit 3ea2ca2 into swiftlang:master Apr 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants