transition borrowck to visit all bodies and not item-likes #39927

nikomatsakis · 2017-02-18T12:08:30Z

This is a better structure for incremental compilation and also more compatible with the eventual borrowck mir. It also fixes #38520 as a drive-by fix.

r? @eddyb

eddyb · 2017-02-18T12:17:02Z

@bors r+

bors · 2017-02-18T12:17:03Z

📌 Commit 4ca1f02 has been approved by eddyb

nikomatsakis · 2017-02-18T12:35:40Z

@bors r=eddyb

bors · 2017-02-18T12:35:41Z

📌 Commit d83d83e has been approved by eddyb

nikomatsakis · 2017-02-19T10:16:46Z

@bors r-

eddyb · 2017-02-21T02:29:21Z

src/librustc/hir/map/mod.rs

@@ -168,43 +168,48 @@ impl<'hir> MapEntry<'hir> {
        })
    }

-    fn is_body_owner(self, node_id: NodeId) -> bool {
+    fn associated_body(self) -> Option<BodyId> {


I have at least two, if not three, reimplementations of this, seems so obvious in retrospect!

eddyb · 2017-02-21T02:32:09Z

src/librustc/dep_graph/visit.rs

@@ -78,9 +79,30 @@ pub fn visit_all_item_likes_in_krate<'a, 'tcx, V, F>(tcx: TyCtxt<'a, 'tcx, 'tcx>
 pub fn visit_all_bodies_in_krate<'a, 'tcx, C>(tcx: TyCtxt<'a, 'tcx, 'tcx>, callback: C)
    where C: Fn(/* body_owner */ DefId, /* body id */ hir::BodyId),
 {
+    // NB: we use a visitor here rather than walking the keys of the
+    // hashmap so as to ensure we visit the bodies "in order".


Well, first off, the hashmap should maybe be replaced with a BTreeMap.
But also, you don't want HIR lowering node IDs to matter... what if it was keyed by the owner's DefIndex?

Either way, BTreeMap might be enough to fix your problem here.

I tried that first, but it still came in what seemed to me to be a surprising order in some cases. But maybe I'll switch it back to a btree-map and just adjust the reference files.

Still, I was thinking that longer term it might be a better idea to have some vector stored in the crate of the preferred "visit order".

I agree, and I'd be fine sorting by Span or something (so the HIR visit order doesn't matter).

nikomatsakis · 2017-02-21T15:38:58Z

I did a crater run of an earlier draft of this branch. Result: 4 false positives, but no regressions found.

nikomatsakis · 2017-02-21T19:07:29Z

@bors r=eddyb

bors · 2017-02-21T19:07:39Z

📌 Commit fed58f3 has been approved by eddyb

nikomatsakis · 2017-02-21T19:08:56Z

@bors r-

I want to wait for my new crater run to complete, actually. And maybe @eddyb wants to take a peek anyhow.

eddyb

r=me

nikomatsakis · 2017-02-22T11:00:41Z

A revised crater run revealed only timeouts (7 of them).

nikomatsakis · 2017-02-22T11:00:45Z

@bors r=eddyb

bors · 2017-02-22T11:00:46Z

📌 Commit fed58f3 has been approved by eddyb

@eddyb

…k-2, r=eddyb transition borrowck to visit all **bodies** and not item-likes This is a better structure for incremental compilation and also more compatible with the eventual borrowck mir. It also fixes rust-lang#38520 as a drive-by fix. r? @eddyb

Rollup of 17 pull requests - Successful merges: #39777, #39815, #39845, #39886, #39892, #39903, #39905, #39914, #39927, #39940, #40010, #40030, #40048, #40050, #40052, #40060, #40071 - Failed merges:

Rollup of 28 pull requests - Successful merges: #39859, #39864, #39888, #39903, #39905, #39914, #39945, #39950, #39953, #39961, #39980, #39988, #39993, #39995, #40019, #40020, #40022, #40024, #40025, #40026, #40027, #40031, #40035, #40037, #40038, #40064, #40069, #40086 - Failed merges: #39927, #40008, #40047

eddyb · 2017-02-28T17:57:50Z

src/librustc_typeck/check/mod.rs

@@ -550,43 +548,6 @@ impl<'a, 'tcx> Visitor<'tcx> for CheckItemTypesVisitor<'a, 'tcx> {
    }
 }

-impl<'a, 'tcx> ItemLikeVisitor<'tcx> for CheckItemBodiesVisitor<'a, 'tcx> {


You should also remove the item_tables queries from CheckItemTypesVisitor (the rest of which belongs in collect IMO).

nikomatsakis · 2017-02-28T21:11:24Z

@eddyb see latest commit, which I think is what you meant

nikomatsakis · 2017-02-28T21:30:19Z

@bors r=eddyb

bors · 2017-02-28T21:30:20Z

📌 Commit f704ef5 has been approved by eddyb

bors · 2017-02-28T22:21:40Z

⌛ Testing commit f704ef5 with merge a1f6823...

bors · 2017-02-28T22:23:04Z

💔 Test failed - status-appveyor

alexcrichton · 2017-02-28T22:30:43Z

@bors: retry * network error

…

On Tue, Feb 28, 2017 at 4:23 PM, bors ***@***.***> wrote: 💔 Test failed - status-appveyor <https://ci.appveyor.com/project/rust-lang/rust/build/1.0.2169> — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#39927 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAD95PU6_DB2HIhAGOfUr1aHORFhoZMmks5rhJ5KgaJpZM4MFGkh> .

nikomatsakis · 2017-03-01T09:49:15Z

@alexcrichton thanks for staying on top of these ... almost every time I find one, you've already tagged it...

bors · 2017-03-01T15:00:21Z

⌛ Testing commit f704ef5 with merge 53f60d5...

bors · 2017-03-01T16:00:49Z

💔 Test failed - status-appveyor

arielb1 · 2017-03-01T16:54:14Z

actual failure:

compile-fail\issue-29184.rs
{"message":"src\\librustc\\hir\\map/mod.rs:379: local_def_id: no entry for `10`, which has a map of `Some(EntryExpr(NodeId(9), expr(10: 92)))`","code":null,"level":"error: internal compiler error","spans":[],"children":[],"rendered":null}

nikomatsakis · 2017-03-01T22:08:38Z

@eddyb r? the last commit

eddyb · 2017-03-01T22:09:35Z

@bors r+

bors · 2017-03-01T22:09:37Z

📌 Commit d572aa2 has been approved by eddyb

bors · 2017-03-03T00:14:15Z

⌛ Testing commit d572aa2 with merge 06c63f6...

@eddyb

transition borrowck to visit all **bodies** and not item-likes This is a better structure for incremental compilation and also more compatible with the eventual borrowck mir. It also fixes #38520 as a drive-by fix. r? @eddyb

bors · 2017-03-03T02:29:48Z

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

kumbayo · 2017-03-03T17:40:40Z

src/librustc_mir/mir_map.rs

@@ -180,23 +181,6 @@ impl<'a, 'gcx: 'tcx, 'tcx> MutVisitor<'tcx> for GlobalizeMir<'a, 'gcx> {
 ///////////////////////////////////////////////////////////////////////////
 // BuildMir -- walks a crate, looking for fn items and methods to build MIR from


It seems this comment refers to the removed BuildMir struct.
Should it also be removed?

rust-highfive assigned eddyb Feb 18, 2017

nikomatsakis force-pushed the incr-comp-skip-borrowck-2 branch 2 times, most recently from d38b5f7 to d83d83e Compare February 18, 2017 12:34

nikomatsakis force-pushed the incr-comp-skip-borrowck-2 branch 2 times, most recently from aabf3df to 7770e9a Compare February 20, 2017 02:58

eddyb reviewed Feb 21, 2017

View reviewed changes

eddyb approved these changes Feb 21, 2017

View reviewed changes

frewsxcv mentioned this pull request Feb 24, 2017

Rollup of 17 pull requests #40080

Closed

eddyb mentioned this pull request Feb 25, 2017

Rollup of 29 pull requests #40089

Closed

eddyb mentioned this pull request Feb 25, 2017

Rollup of 28 pull requests #40091

Merged

convert MIR to iterate over the bodies vector

384f044

eddyb reviewed Feb 28, 2017

View reviewed changes

simplify check-item-types too

f704ef5

fix tests to handle the Typeof bodies

d572aa2

frewsxcv mentioned this pull request Mar 2, 2017

Rollup of 6 pull requests #40218

Closed

bors merged commit d572aa2 into rust-lang:master Mar 3, 2017

bors mentioned this pull request Mar 3, 2017

syntax: add ast::ItemKind::MacroDef, simplify hygiene info #40220

Merged

kumbayo reviewed Mar 3, 2017

View reviewed changes

nikomatsakis deleted the incr-comp-skip-borrowck-2 branch April 14, 2017 10:13

kennytm mentioned this pull request Jun 1, 2017

error: internal compiler error: /checkout/src/librustc_borrowck/borrowck/mod.rs:808: aliasability violation for static cannot borrow data mutably #42344

Closed

		@@ -180,23 +181,6 @@ impl<'a, 'gcx: 'tcx, 'tcx> MutVisitor<'tcx> for GlobalizeMir<'a, 'gcx> {
		///////////////////////////////////////////////////////////////////////////
		// BuildMir -- walks a crate, looking for fn items and methods to build MIR from

transition borrowck to visit all **bodies** and not item-likes #39927

transition borrowck to visit all **bodies** and not item-likes #39927

Conversation

nikomatsakis commented Feb 18, 2017

eddyb commented Feb 18, 2017

bors commented Feb 18, 2017

nikomatsakis commented Feb 18, 2017

bors commented Feb 18, 2017

nikomatsakis commented Feb 19, 2017

eddyb Feb 21, 2017

Choose a reason for hiding this comment

eddyb Feb 21, 2017

Choose a reason for hiding this comment

eddyb Feb 21, 2017

Choose a reason for hiding this comment

nikomatsakis Feb 21, 2017

Choose a reason for hiding this comment

eddyb Feb 21, 2017

Choose a reason for hiding this comment

nikomatsakis commented Feb 21, 2017

nikomatsakis commented Feb 21, 2017

bors commented Feb 21, 2017

nikomatsakis commented Feb 21, 2017

eddyb left a comment

Choose a reason for hiding this comment

nikomatsakis commented Feb 22, 2017

nikomatsakis commented Feb 22, 2017

bors commented Feb 22, 2017

eddyb Feb 28, 2017

Choose a reason for hiding this comment

nikomatsakis commented Feb 28, 2017

nikomatsakis commented Feb 28, 2017

bors commented Feb 28, 2017

bors commented Feb 28, 2017

bors commented Feb 28, 2017

alexcrichton commented Feb 28, 2017 via email

nikomatsakis commented Mar 1, 2017

bors commented Mar 1, 2017

bors commented Mar 1, 2017

arielb1 commented Mar 1, 2017

nikomatsakis commented Mar 1, 2017

eddyb commented Mar 1, 2017

bors commented Mar 1, 2017

bors commented Mar 3, 2017

bors commented Mar 3, 2017

kumbayo Mar 3, 2017

Choose a reason for hiding this comment

transition borrowck to visit all bodies and not item-likes #39927

transition borrowck to visit all bodies and not item-likes #39927