Various improvements to the SVH #35079

nikomatsakis · 2016-07-27T21:33:24Z

This fixes a few points for the SVH:

incorporate resolve results into the SVH;
don't include nested items.

r? @michaelwoerister

cc #32753 (not fully fixed I don't think)

nikomatsakis · 2016-07-27T21:34:13Z

we may need to add a few more resolve cases, actually.

michaelwoerister · 2016-07-28T08:10:46Z

src/test/incremental/ich_nested_items.rs

+// except according to those terms.
+
+// Check that the hash for `mod3::bar` changes when we change the
+// `use` to something different.


This comment seems to be a leftover from another test case.

michaelwoerister · 2016-07-28T08:30:46Z

Looks good to me. I like that we are getting more and more of these test cases that make sure some fundamental things don't regress.

we may need to add a few more resolve cases, actually.

Care to elaborate on that?

nikomatsakis · 2016-07-28T16:06:01Z

@michaelwoerister re-r? those last few commits -- I switched to use IdVisitor

bors · 2016-07-29T00:17:19Z

☔ The latest upstream changes (presumably #34956) made this pull request unmergeable. Please resolve the merge conflicts.

michaelwoerister · 2016-07-29T13:02:47Z

src/librustc_incremental/calculate_svh/mod.rs

@@ -58,6 +58,7 @@ impl<'a, 'tcx> SvhCalculate for TyCtxt<'a, 'tcx, 'tcx> {
        {
            let mut visit = StrictVersionHashVisitor::new(&mut state, self);
            krate.visit_all_items(&mut visit);
+            krate.visit_all_items(&mut IdVisitor::new(&mut visit));


We probably don't need to do this for the SVH since we are visiting all use declarations too in that case, right? Although, that would kind of rely on things being visited in lexical order...

michaelwoerister · 2016-07-29T13:07:55Z

The new commits look good. However, the IdVisitor is on it's way out (see #35090). But with that refactoring landed it should be even easier, since then the visit_id method will just be part of the regular Visitor trait. We'll see which PR wins the race ;)

nikomatsakis · 2016-07-29T13:11:55Z

We probably don't need to do this for the SVH since we are visiting all use declarations too in that case, right? Although, that would kind of rely on things being visited in lexical order...

Perhaps, though I've been wondering if can just strip use statements out of the HIR. They don't seem needed if resolution checking is taking place earlier.

nikomatsakis · 2016-07-29T13:37:26Z

@michaelwoerister

We'll see which PR wins the race

Game on! ;)

Yes, I expect to rebase on #35090, though this PR has a lower number I guess.

michaelwoerister · 2016-07-29T13:44:30Z

Perhaps, though I've been wondering if can just strip use statements out of the HIR.

We don't do it yet, but we plan to map them into debuginfo as DW_TAG_imported_declaration DIEs, so that's one case where we'd still need them.

nikomatsakis · 2016-07-29T13:50:33Z

@michaelwoerister rebased atop your PR.

@bors r=mw

bors · 2016-07-29T13:50:33Z

📌 Commit 660716b has been approved by mw

nikomatsakis · 2016-07-29T17:59:17Z

@bors r-

Travis failure looks legit:

---- [codegen-units] codegen-units/partitioning/extern-generic.rs stdout ----



error: compilation failed!

status: exit code: 101

command: x86_64-unknown-linux-gnu/stage2/bin/rustc /build/src/test/codegen-units/partitioning/extern-generic.rs -L x86_64-unknown-linux-gnu/test/codegen-units/ --target=x86_64-unknown-linux-gnu -L x86_64-unknown-linux-gnu/test/codegen-units/partitioning/extern-generic.stage2-x86_64-unknown-linux-gnu.codegen-units.libaux -C prefer-dynamic -o x86_64-unknown-linux-gnu/test/codegen-units/partitioning/extern-generic.stage2-x86_64-unknown-linux-gnu --cfg rtopt -C rpath -O -L x86_64-unknown-linux-gnu/rt -Zprint-trans-items=eager -Zincremental=tmp/partitioning-tests/extern-generic

stdout:

------------------------------------------

TRANS_ITEM drop-glue i8 @@ extern_generic[Internal] extern_generic-mod1[Internal] extern_generic-mod1-mod1[Internal] extern_generic-mod2[Internal]

TRANS_ITEM fn cgu_generic_function::bar[0]<&str> @@ extern_generic[Internal] extern_generic-mod1[Internal] extern_generic-mod1-mod1[Internal] extern_generic-mod2[Internal]

TRANS_ITEM fn cgu_generic_function::foo[0]<&str> @@ extern_generic[Internal] extern_generic-mod1[Internal] extern_generic-mod1-mod1[Internal] extern_generic-mod2[Internal]

TRANS_ITEM fn extern_generic::mod1[0]::mod1[0]::user[0] @@ extern_generic-mod1-mod1[External]

TRANS_ITEM fn extern_generic::mod1[0]::user[0] @@ extern_generic-mod1[External]

TRANS_ITEM fn extern_generic::mod2[0]::user[0] @@ extern_generic-mod2[External]

TRANS_ITEM fn extern_generic::mod3[0]::non_user[0] @@ extern_generic-mod3[External]

TRANS_ITEM fn extern_generic::user[0] @@ extern_generic[External]

thread 'rustc' panicked at 'Box<Any>', src/librustc_errors/lib.rs:636

note: Run with `RUST_BACKTRACE=1` for a backtrace.

------------------------------------------

stderr:

------------------------------------------

error: internal compiler error: src/librustc_metadata/decoder.rs:75: lookup_item: id not found: DefIndex(14) in crate "cgu_generic_function" with number 11 

note: the compiler unexpectedly panicked. this is a bug. 

note: we would appreciate a bug report: https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.md#bug-reports 

------------------------------------------

thread '[codegen-units] codegen-units/partitioning/extern-generic.rs' panicked at 'explicit panic', /build/src/tools/compiletest/src/runtest.rs:2353

note: Run with `RUST_BACKTRACE=1` for a backtrace.

failures:

    [codegen-units] codegen-units/partitioning/extern-generic.rs

bors · 2016-07-30T02:08:43Z

☔ The latest upstream changes (presumably #34842) made this pull request unmergeable. Please resolve the merge conflicts.

nikomatsakis · 2016-08-09T18:50:24Z

@bors r=mw

bors · 2016-08-09T18:50:25Z

📌 Commit 257b9a4 has been approved by mw

bors · 2016-08-09T23:49:53Z

⌛ Testing commit 257b9a4 with merge 04e0733...

nikomatsakis · 2016-08-09T23:58:05Z

@bors r-

tidy violations

bors · 2016-08-10T00:00:53Z

⛄ The build was interrupted to prioritize another pull request.

This requires passing in the dirty-node set explicitly since HIR nodes wind up added to the graph either way.

We now incorporate the `def_map` and `trait_map` results into the SVH.

nikomatsakis · 2016-08-10T01:12:10Z

@bors r=mw

bors · 2016-08-10T01:12:11Z

📌 Commit 83068eb has been approved by mw

bors · 2016-08-10T04:00:21Z

⌛ Testing commit 83068eb with merge 561c4e1...

@michaelwoerister

Various improvements to the SVH This fixes a few points for the SVH: - incorporate resolve results into the SVH; - don't include nested items. r? @michaelwoerister cc #32753 (not fully fixed I don't think)

bors · 2016-08-10T06:59:56Z

rust-highfive assigned michaelwoerister Jul 27, 2016

michaelwoerister reviewed Jul 28, 2016
View reviewed changes

michaelwoerister reviewed Jul 29, 2016
View reviewed changes

nikomatsakis force-pushed the incr-comp-ich-32753 branch from ec27189 to 660716b Compare July 29, 2016 13:49

nikomatsakis force-pushed the incr-comp-ich-32753 branch 3 times, most recently from fc694c0 to 257b9a4 Compare August 9, 2016 18:50

nikomatsakis added 3 commits August 9, 2016 20:28

make it possible to test if HIR is dirty

775bd93

This requires passing in the dirty-node set explicitly since HIR nodes wind up added to the graph either way.

stop hashing nested items, and add a test

5001c92

remove field that need not be public

ae9c049

nikomatsakis added 3 commits August 9, 2016 20:28

promote svh calculation into its own directory

953d711

incorporate resolve results into hashing

c7f15aa

We now incorporate the `def_map` and `trait_map` results into the SVH.

pacify the merciless tidy

83068eb

nikomatsakis force-pushed the incr-comp-ich-32753 branch from 257b9a4 to 83068eb Compare August 10, 2016 00:57

bors merged commit 83068eb into rust-lang:master Aug 10, 2016

nikomatsakis deleted the incr-comp-ich-32753 branch October 3, 2016 14:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Various improvements to the SVH #35079

Various improvements to the SVH #35079

nikomatsakis commented Jul 27, 2016

nikomatsakis commented Jul 27, 2016

michaelwoerister Jul 28, 2016

michaelwoerister commented Jul 28, 2016

nikomatsakis commented Jul 28, 2016

bors commented Jul 29, 2016

michaelwoerister Jul 29, 2016

michaelwoerister commented Jul 29, 2016

nikomatsakis commented Jul 29, 2016

nikomatsakis commented Jul 29, 2016

michaelwoerister commented Jul 29, 2016

nikomatsakis commented Jul 29, 2016

bors commented Jul 29, 2016

nikomatsakis commented Jul 29, 2016

bors commented Jul 30, 2016

nikomatsakis commented Aug 9, 2016

bors commented Aug 9, 2016

bors commented Aug 9, 2016

nikomatsakis commented Aug 9, 2016

bors commented Aug 10, 2016

nikomatsakis commented Aug 10, 2016

bors commented Aug 10, 2016

bors commented Aug 10, 2016

bors commented Aug 10, 2016

Various improvements to the SVH #35079

Various improvements to the SVH #35079

Conversation

nikomatsakis commented Jul 27, 2016

nikomatsakis commented Jul 27, 2016

michaelwoerister Jul 28, 2016

Choose a reason for hiding this comment

michaelwoerister commented Jul 28, 2016

nikomatsakis commented Jul 28, 2016

bors commented Jul 29, 2016

michaelwoerister Jul 29, 2016

Choose a reason for hiding this comment

michaelwoerister commented Jul 29, 2016

nikomatsakis commented Jul 29, 2016

nikomatsakis commented Jul 29, 2016

michaelwoerister commented Jul 29, 2016

nikomatsakis commented Jul 29, 2016

bors commented Jul 29, 2016

nikomatsakis commented Jul 29, 2016

bors commented Jul 30, 2016

nikomatsakis commented Aug 9, 2016

bors commented Aug 9, 2016

bors commented Aug 9, 2016

nikomatsakis commented Aug 9, 2016

bors commented Aug 10, 2016

nikomatsakis commented Aug 10, 2016

bors commented Aug 10, 2016

bors commented Aug 10, 2016

bors commented Aug 10, 2016