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

fix regressions on assignment expressions #12680

Merged
merged 2 commits into from Jul 2, 2022

Conversation

lowr
Copy link
Contributor

@lowr lowr commented Jul 2, 2022

This is a follow-up PR on #12428. I'm not sure if this is everything I overlooked, so if there are more things that are not right, we may want to revert #12428.

This should also fix the increase of the type mismatches and the unknown types in diesel in the metrics introduced by #12428.

The regressions are:

  • some coercions don't work in the ordinary (i.e. non-destructuring) assignments

    In order for coercions on ADT fields instantiations to work, lhs type has to be known before inferring rhs. feat: implement destructuring assignment #12428 changed the inference order, making rhs inferred before lhs, breaking the coercion, so I restored the original inference mechanism for the ordinary assignments.

    Note that this kind of coercion doesn't happen in destructuring assigments, because when they are desugared, the struct expression is first assigned to a temporary, which is then assigned to the assignee, which is not coercion site anymore.

  • type mismatches on individual identifiers are not reported

@Veykril
Copy link
Member

Veykril commented Jul 2, 2022

Thanks!
@bors r+

@bors
Copy link
Collaborator

bors commented Jul 2, 2022

📌 Commit 649e1f5 has been approved by Veykril

@bors
Copy link
Collaborator

bors commented Jul 2, 2022

⌛ Testing commit 649e1f5 with merge d1ac462...

@bors
Copy link
Collaborator

bors commented Jul 2, 2022

☀️ Test successful - checks-actions
Approved by: Veykril
Pushing d1ac462 to master...

@bors bors merged commit d1ac462 into rust-lang:master Jul 2, 2022
@lowr
Copy link
Contributor Author

lowr commented Jul 3, 2022

changelog skip

I think this shouldn't be on the changelog, as #12428 hasn't been released yet, but if maintainers think otherwise, please do edit this comment.

@lnicola
Copy link
Member

lnicola commented Jul 3, 2022

I think this shouldn't be on the changelog, as #12428 hasn't been released yet, but if maintainers think otherwise, please do edit this comment.

We sometimes include fixup PRs together with the original one.

changelog fix fixup for 12428

@lowr
Copy link
Contributor Author

lowr commented Jul 3, 2022

Got it, thanks!

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.

None yet

4 participants