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

[NLL] Fix some things for bootstrap #52830

Merged
merged 2 commits into from
Jul 30, 2018
Merged

Conversation

matthewjasper
Copy link
Contributor

Some changes that are required when bootstrapping rustc with NLL enabled.

  • Remove a bunch of unused muts that aren't needed, but the existing lint doesn't catch.
  • Rewrite a function call to satisfy NLL borrowck. Note that the borrow is two-phase, but gets activated immediately by an unsizing coercion.

cc #51823

Note that the first argument is `self as &mut dyn Delegate`, so this
isn't allowed with two-phase borrows.
@rust-highfive
Copy link
Collaborator

r? @varkor

(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 29, 2018
@@ -56,7 +56,7 @@ impl<K: Ord, V> SortedMap<K, V> {
pub fn insert(&mut self, key: K, mut value: V) -> Option<V> {
match self.lookup_index_for(&key) {
Ok(index) => {
let mut slot = unsafe {
let slot = unsafe {
self.data.get_unchecked_mut(index)
};
mem::swap(&mut slot.1, &mut value);
Copy link
Member

Choose a reason for hiding this comment

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

I'm surprised this doesn't require mut on the variable; generally &mut <var> does require <var> to be declared mut I think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, but this is equivalent to &mut (*slot).1 which is OK since slot is a mutable reference.

@Mark-Simulacrum
Copy link
Member

@bors delegate+

r=me pending travis

@bors
Copy link
Contributor

bors commented Jul 29, 2018

✌️ @matthewjasper can now approve this pull request

@matthewjasper
Copy link
Contributor Author

@bors r+

@bors
Copy link
Contributor

bors commented Jul 29, 2018

📌 Commit 18d5f82 has been approved by matthewjasper

@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 29, 2018
@bors
Copy link
Contributor

bors commented Jul 30, 2018

⌛ Testing commit 18d5f82 with merge e437841...

bors added a commit that referenced this pull request Jul 30, 2018
[NLL] Fix some things for bootstrap

Some changes that are required when bootstrapping rustc with NLL enabled.

* Remove a bunch of unused `mut`s that aren't needed, but the existing lint doesn't catch.
* Rewrite a function call to satisfy NLL borrowck. Note that the borrow is two-phase, but gets activated immediately by an unsizing coercion.

cc #51823
@bors
Copy link
Contributor

bors commented Jul 30, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: matthewjasper
Pushing e437841 to master...

@bors bors merged commit 18d5f82 into rust-lang:master Jul 30, 2018
@@ -248,7 +248,8 @@ impl<'a, 'tcx> CheckCrateVisitor<'a, 'tcx> {
let tcx = self.tcx;
let param_env = self.param_env;
let region_scope_tree = self.tcx.region_scope_tree(item_def_id);
euv::ExprUseVisitor::new(self, tcx, param_env, &region_scope_tree, self.tables, None)
let tables = self.tables;
euv::ExprUseVisitor::new(self, tcx, param_env, &region_scope_tree, tables, None)
Copy link
Member

@pnkfelix pnkfelix Jul 31, 2018

Choose a reason for hiding this comment

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

hmm we may want to actually file an issue for this one (the interaction of two-phase borrows and an Unsize coercion), tagged with NLL, in terms of tracking things that regress with respect to AST-borrowck.

Or we can just wait to someone to report it from the wild. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's already #51915

@matthewjasper matthewjasper deleted the bootstrap-prep branch July 31, 2018 22:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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