Skip to content

[Sema] Better key path failure diagnosis for unresolved roots #18329

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 2 commits into from
Aug 1, 2018

Conversation

gregomni
Copy link
Contributor

When the root type of a keypath is unresolved by the constraint system, use the root interface type for diagnosis.

Resolves SR-7339.

@gregomni gregomni requested a review from rudkx July 28, 2018 15:23
@gregomni gregomni changed the title [Sema] Better key path failure diagnosis for generic roots [Sema] Better key path failure diagnosis for unresolved roots Jul 28, 2018
@rudkx rudkx requested a review from xedin July 29, 2018 00:25
@xedin
Copy link
Contributor

xedin commented Jul 29, 2018

@gregomni Sorry I'm out for a bit, going to take a look tonight!

Copy link
Contributor

@xedin xedin left a comment

Choose a reason for hiding this comment

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

LGTM with a nit, definitely better diagnostic than what it is right now...

var here: (() -> Void)?
}
let some = Some(keyPath: \Demo.here)
// expected-error@-1 {{cannot convert value of type 'ReferenceWritableKeyPath<Demo, (() -> Void)?>' to expected argument type 'KeyPath<_, ((_) -> Void)?>'}}
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please add a FIXME here to improve this diagnostic, it's better in a way that points to a correct place but doesn't really pinpoint the problem unfortunately still...

@gregomni
Copy link
Contributor Author

@xedin Sure thing, thanks!

@gregomni
Copy link
Contributor Author

@swift-ci Please smoke test and merge.

@gregomni
Copy link
Contributor Author

gregomni commented Aug 1, 2018

@swift-ci Please smoke test.

@gregomni
Copy link
Contributor Author

gregomni commented Aug 1, 2018

@swift-ci Please smoke test.

@gregomni gregomni merged commit 5e7da0e into swiftlang:master Aug 1, 2018
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.

2 participants