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

Some preparatory refactorings for hash-based DepNodes #42504

Merged
merged 4 commits into from
Jun 9, 2017

Conversation

michaelwoerister
Copy link
Member

This PR collects some changes that turn out to be necessary for implementing DepNodes based on stable hashes (see #42294). The commits are self-contained and mostly straightforward.

The most interesting change here is the introduction of DefIndices for things that are not part of the AST: Some pieces of crate metadata now have a DefIndex too.

cc @eddyb
r? @nikomatsakis

…n being able to reconstruct obj-file names from one.
This allows for treating global crate metadata the same as regular metadata with regard to incr. comp.
@@ -56,7 +58,7 @@ pub enum DepNode<D: Clone + Debug> {

/// Represents some artifact that we save to disk. Note that these
/// do not have a def-id as part of their identifier.
WorkProduct(Arc<WorkProductId>),
WorkProduct(WorkProductId),
Copy link
Contributor

Choose a reason for hiding this comment

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

does this make DepNode be Copy? That would be great!

Copy link
Contributor

Choose a reason for hiding this comment

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

(What is Fingerprint anyway?)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, DepNode will be Copy.
A Fingerprint is a hash value with a "virtual uniqueness" property, i.e. there will be collisions (like with any hash) but the probability is so low that it doesn't matter. Because of this property, a Fingerprint can be used as an identifier. In terms of rustc I now use the type Fingerprint for anything that fulfills this property but sometimes I like to introduce newtypes for it, for example DefPathHash.

@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Jun 7, 2017

📌 Commit 8b36d33 has been approved by nikomatsakis

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

bors commented Jun 9, 2017

⌛ Testing commit 8b36d33 with merge 19193d6...

bors added a commit that referenced this pull request Jun 9, 2017
…tsakis

Some preparatory refactorings for hash-based DepNodes

This PR collects some changes that turn out to be necessary for implementing `DepNodes` based on stable hashes (see #42294). The commits are self-contained and mostly straightforward.

The most interesting change here is the introduction of `DefIndices` for things that are not part of the AST: Some pieces of crate metadata now have a `DefIndex` too.

cc @eddyb
r? @nikomatsakis
@bors
Copy link
Contributor

bors commented Jun 9, 2017

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

@bors bors merged commit 8b36d33 into rust-lang:master Jun 9, 2017
bors added a commit that referenced this pull request Jun 12, 2017
incr.comp.: Make DepNode `Copy` and valid across compilation sessions

This PR moves `DepNode` to a representation that does not need retracing and thus simplifies comparing dep-graphs from different compilation sessions. The code also gets a lot simpler in many places, since we don't need the generic parameter on `DepNode` anymore.  See #42294 for details.

~~NOTE: Only the last commit of this is new, the rest is already reviewed in #42504

This PR is almost done but there are some things I still want to do:
- [x] Add some module-level documentation to `dep_node.rs`, explaining especially what the `define_dep_nodes!()` macro is about.
- [x] Do another pass over the dep-graph loading logic. I suspect that we can get rid of building the `edges` map and also use arrays instead of hash maps in some places.

cc @rust-lang/compiler
r? @nikomatsakis
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.

4 participants