Skip to content

[NFC] Never synthesize @lvalue in typing judgements #16856

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
May 29, 2018

Conversation

CodaFi
Copy link
Contributor

@CodaFi CodaFi commented May 26, 2018

When in a checking context, there is no need (and no sense) in synthesizing @lvalue instead of checking the underlying object type.

The locators for this typing rule and the other parameter binding one below it are still lying, but probably because there's really no good way to indicate that you're an argument to a function/operator without also lying about the index of the argument - which matchTypes has no way of knowing.

@CodaFi CodaFi requested a review from DougGregor May 26, 2018 00:13
@CodaFi
Copy link
Contributor Author

CodaFi commented May 26, 2018

@swift-ci please test

@CodaFi
Copy link
Contributor Author

CodaFi commented May 26, 2018

@swift-ci please test source compatibility

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - 056ddca2417e92f435043ee98de41a65cfb3c308

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.

I think this is reasonable, but it looks like it might require special handling for mutable keypaths?

@CodaFi
Copy link
Contributor Author

CodaFi commented May 26, 2018

Aha, well I'm going to move the @lvalue synthesis to CSGen where it belongs.

@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - 056ddca2417e92f435043ee98de41a65cfb3c308

@CodaFi
Copy link
Contributor Author

CodaFi commented May 26, 2018

Scaling back to just the rule in matchTypes - attacking the keypath rule makes this patch unruly.

@swift-ci please smoke test

Only two more of these exist
1) CSGen's formation of @lvalue -> inout conversion constraints
2) WritableKeyPath's checking rules

Both can be eliminated but will require more refactoring.
@CodaFi
Copy link
Contributor Author

CodaFi commented May 27, 2018

LLDB test timed out

@swift-ci please smoke test

@CodaFi
Copy link
Contributor Author

CodaFi commented May 28, 2018

@swift-ci please test source compatibility

@CodaFi
Copy link
Contributor Author

CodaFi commented May 29, 2018

⛵️

@CodaFi CodaFi merged commit 96c8f9b into swiftlang:master May 29, 2018
@CodaFi CodaFi deleted the formica branch May 29, 2018 23:22
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