Skip to content

Conversation

michaelwoerister
Copy link
Member

With #42537 a regular DepNode only contains an opaque hash as its identifier. In most cases, this hash is actually a DefPathHash and we can reconstruct the DefId it came from via a table lookup --- and then use that to print something intelligible for debug outputs. For cases where we cannot reconstruct information from the DepNode's hash, this PR will cache a string representation of the DepNode in a side-table. This string is later used for debug outputs.

r? @nikomatsakis

@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Collaborator

bors commented Jun 13, 2017

📌 Commit 5b5499d has been approved by nikomatsakis

@nikomatsakis
Copy link
Contributor

@bors r-

let (def_id_0, def_id_1) = *self;

let def_path_hash_0 = tcx.def_path_hash(def_id_0);
let def_path_hash_1 = tcx.def_path_hash(def_id_1);
Copy link
Contributor

Choose a reason for hiding this comment

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

it'd be good to add a comment clarifying that this is a micro-opt, and the generic impl also suffices

@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Collaborator

bors commented Jun 13, 2017

📌 Commit e323652 has been approved by nikomatsakis

@arielb1 arielb1 added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jun 13, 2017
@bors
Copy link
Collaborator

bors commented Jun 14, 2017

⌛ Testing commit e323652 with merge 9430800...

@bors
Copy link
Collaborator

bors commented Jun 14, 2017

💔 Test failed - status-travis

@michaelwoerister
Copy link
Member Author

@bors retry

(looks like a time-out)

@bors
Copy link
Collaborator

bors commented Jun 14, 2017

⌛ Testing commit e323652 with merge aaf685b...

bors added a commit that referenced this pull request Jun 14, 2017
incr.comp.: Make DepNode's std::fmt::Debug implementation useful again.

With #42537 a regular `DepNode` only contains an opaque hash as its identifier. In most cases, this hash is actually a `DefPathHash` and we can reconstruct the `DefId` it came from via a table lookup --- and then use that to print something intelligible for debug outputs. For cases where we cannot reconstruct information from the DepNode's hash, this PR will cache a string representation of the `DepNode` in a side-table. This string is later used for debug outputs.

r? @nikomatsakis
@bors
Copy link
Collaborator

bors commented Jun 14, 2017

💔 Test failed - status-travis

@Mark-Simulacrum
Copy link
Member

@bors retry

  • timeout

@alexcrichton
Copy link
Member

Hm so the timed out build compared to the previous successful build shows that stage0-rustc jumped from 1000s to 2600s and stage1-rustc jumped from 1300s to 3500s.

This PR doesn't seem particularly at fault, I'm messaging Travis to hopefully clarify

@bors
Copy link
Collaborator

bors commented Jun 15, 2017

⌛ Testing commit e323652 with merge 119066f...

bors added a commit that referenced this pull request Jun 15, 2017
incr.comp.: Make DepNode's std::fmt::Debug implementation useful again.

With #42537 a regular `DepNode` only contains an opaque hash as its identifier. In most cases, this hash is actually a `DefPathHash` and we can reconstruct the `DefId` it came from via a table lookup --- and then use that to print something intelligible for debug outputs. For cases where we cannot reconstruct information from the DepNode's hash, this PR will cache a string representation of the `DepNode` in a side-table. This string is later used for debug outputs.

r? @nikomatsakis
@bors
Copy link
Collaborator

bors commented Jun 15, 2017

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

@bors bors merged commit e323652 into rust-lang:master Jun 15, 2017
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