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

rustc_typeck: do not overlap a borrow of TypeckTables with method lookup. #42480

Merged
merged 1 commit into from
Jun 7, 2017

Conversation

eddyb
Copy link
Member

@eddyb eddyb commented Jun 6, 2017

If trait selection is reached, it could potentially request a closure signature, which will have to borrow the TypeckTables of the current function, and so those tables should not be mutably borrowed.

Fixes #42463.
r? @nikomatsakis

@eddyb eddyb requested a review from nikomatsakis June 6, 2017 15:53
@eddyb eddyb added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jun 6, 2017
@eddyb
Copy link
Member Author

eddyb commented Jun 6, 2017

Nominating for backport because the bug has already reached beta.

Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

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

r=me w/ or w/o comment

@@ -446,9 +446,10 @@ impl<'a, 'gcx, 'tcx> ConfirmContext<'a, 'gcx, 'tcx> {
// overloaded lvalue ops, and will be fixed by them in order to get
// the correct region.
let mut source = self.node_ty(expr.id);
if let Some(adjustments) = self.tables.borrow_mut().adjustments.get_mut(&expr.id) {
let previous_adjustments = self.tables.borrow_mut().adjustments.remove(&expr.id);
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe worth a comment here explaining why the remove-and-then-reinsert?

@eddyb
Copy link
Member Author

eddyb commented Jun 6, 2017

@bors r=nikomatsakis

@bors
Copy link
Contributor

bors commented Jun 6, 2017

📌 Commit b02e3a1 has been approved by nikomatsakis

@bors
Copy link
Contributor

bors commented Jun 7, 2017

⌛ Testing commit b02e3a1 with merge 89fceaa...

bors added a commit that referenced this pull request Jun 7, 2017
rustc_typeck: do not overlap a borrow of TypeckTables with method lookup.

If trait selection is reached, it could potentially request a closure signature, which will have to borrow the `TypeckTables` of the current function, and so those tables *should not* be mutably borrowed.

Fixes #42463.
r? @nikomatsakis
@bors
Copy link
Contributor

bors commented Jun 7, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: nikomatsakis
Pushing 89fceaa to master...

@bors bors merged commit b02e3a1 into rust-lang:master Jun 7, 2017
@eddyb eddyb deleted the issue-42463 branch June 7, 2017 10:36
@brson brson added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Jun 12, 2017
@brson brson mentioned this pull request Jun 12, 2017
@brson brson removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jun 13, 2017
bors added a commit that referenced this pull request Jun 15, 2017
Beta next

- #42521
- #42512
- #42482
- #42481
- #42480

r? @nikomatsakis remember to untag 'beta-nominated' on linked issues
bors added a commit that referenced this pull request Aug 19, 2017
[beta] back out #42480 and its dependents

#42480 makes the ICE in #43132 worse, and the "safe" fix to that causes the #43787 exponential worst-case.

Let's back everything out for beta to avoid regressions, and get the permafix in nightly.
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants