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

LHS of assign op is invariant #52564

Merged

Conversation

pnkfelix
Copy link
Member

This addresses a bug injected by #45435. That PR changed the way we type-check LHS <op> RHS to coerce the LHS to the expected supertype in much the same way that we coerce the RHS.

The problem is that when we have a case of LHS <op>= RHS, we do not want to coerce to a supertype; we need the type to remain invariant. Otherwise we risk leaking a value with short-lifetimes into a expression context that needs to satisfy a long lifetime.

Fix #52126

pnkfelix added 3 commits July 20, 2018 13:12
Therefore we cannot coerce it to a supertype the same way that we can
the LHS of `+`.

Addresses issue 52126.
… linker failure.

Rather than try to work out what was happening, I just removed the flag because
I see no reason for it to be on this test.
@rust-highfive
Copy link
Collaborator

r? @cramertj

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jul 20, 2018
@pnkfelix pnkfelix added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jul 20, 2018
@pnkfelix
Copy link
Member Author

r? @eddyb

@rust-highfive rust-highfive assigned eddyb and unassigned cramertj Jul 20, 2018
@eddyb
Copy link
Member

eddyb commented Jul 20, 2018

/facepalm of course (I probably completely missed that OP= went through the same codepath)

@bors r+

@bors
Copy link
Contributor

bors commented Jul 20, 2018

📌 Commit f153be6 has been approved by eddyb

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 20, 2018
@bors
Copy link
Contributor

bors commented Jul 22, 2018

⌛ Testing commit f153be6 with merge 67f9c71...

bors added a commit that referenced this pull request Jul 22, 2018
…iant, r=eddyb

LHS of assign op is invariant

This addresses a bug injected by #45435. That PR changed the way we type-check `LHS <op> RHS` to coerce the LHS to the expected supertype in much the same way that we coerce the RHS.

The problem is that when we have a case of `LHS <op>= RHS`, we do not want to coerce to a supertype; we need the type to remain invariant. Otherwise we risk leaking a value with short-lifetimes into a expression context that needs to satisfy a long lifetime.

Fix #52126
@bors
Copy link
Contributor

bors commented Jul 23, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: eddyb
Pushing 67f9c71 to master...

@bors bors merged commit f153be6 into rust-lang:master Jul 23, 2018
@pnkfelix pnkfelix added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Jul 26, 2018
@pietroalbini pietroalbini removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jul 26, 2018
bors added a commit that referenced this pull request Jul 26, 2018
[beta] Rollup backports

Merged and approved:

* #52564: LHS of assign op is invariant

This PR also reverts commit 3fd3221, which is not needed anymore. cc @kennytm

r? @ghost
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants