Skip to content

Don't use external node ids in crate metadata #2419

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

Closed
brson opened this issue May 22, 2012 · 8 comments
Closed

Don't use external node ids in crate metadata #2419

brson opened this issue May 22, 2012 · 8 comments
Labels
A-linkage Area: linking into static, shared libraries and binaries P-low Low priority

Comments

@brson
Copy link
Contributor

brson commented May 22, 2012

I believe one of the reasons linking is so flaky right now is because we encode data about external crates using their external node ids, which are not stable.

For example if crate A uses type foo from crate B, when we write A's metadata we will refer to foo as node 3048 (for example). When B is recompiled foo will probably get a different node ID, while A still refers to the old ID.

When we later link a different crate to crate A the metadata reader runs off the rails chasing bogus node ID's in crate B.

Here's what I think is the easiest way to fix this within the current design:

  1. define a new cryptographic hash, node_hash, that hashes the type signature and path of a node
  2. when we write a crate's metadata, we also write a table that maps from the node hash to the node id
  3. when we write a crate's metadata, we also write a table that maps all used external def ids to node hashes
  4. when looking up a node in crate B via crate A we first translate A's notion of B's node id to the node hash, then look up the node hash in crate B to find the actual, current node id.
@catamorphism
Copy link
Contributor

Nominating for milestone 5, production-ready

@pcwalton
Copy link
Contributor

pcwalton commented Jun 6, 2013

We're calling this "def ID drift" and it's disrupting Servo a lot.

@graydon
Copy link
Contributor

graydon commented Jun 6, 2013

accepted for production-ready milestone

@pnkfelix
Copy link
Member

Accepted for P-low.

@metajack
Copy link
Contributor

This does not seem to be a problem for Servo these days.

@emberian
Copy link
Member

Rust now gives better warnings when libraries start to drift out of date, and I haven't observed def id drift in practice for quite some time now. This is still an issue, though.

@steveklabnik
Copy link
Member

A lot has changed in a post-cargo world. Is this still true? Can we get a reproduction?

@arielb1
Copy link
Contributor

arielb1 commented Jun 12, 2015

We now reference dependencies by-hash, making relying on internal NodeIds perfectly fine (that's it, until we decide on a [stable-ey] metadata ABI).

@arielb1 arielb1 closed this as completed Jun 12, 2015
celinval pushed a commit to celinval/rust-dev that referenced this issue Jun 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-linkage Area: linking into static, shared libraries and binaries P-low Low priority
Projects
None yet
Development

No branches or pull requests

9 participants