Rollup of 6 pull requests by frewsxcv · Pull Request #40752 · rust-lang/rust (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation5 Commits24 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

replace unwrap() with expect()

HirId has a more stable representation than NodeId, meaning that modifications to one item don't influence (part of) the IDs within other items. The other part is a DefIndex for which there already is a way of stable hashing and persistence.

This commit introduces the HirId type and generates a HirId for every NodeId during HIR lowering, but the resulting values are not yet used anywhere, except in consistency checks.

This way we can have all item-likes occupy a dense range of DefIndexes, which is good for making fast, array-based dictionaries.

Teach rustc --emit=mir

I'm opening this PR to discuss:

Is this a good idea?
Is this a good implementation?

I'm sure people will have opinions on both points!

This spawned from rust-lang#31847 (comment), so I figured a prototype implementation could help provide a seed to talk about.

Introduce HirId, a replacement for ast::NodeId after lowering to HIR

This is the first step towards implementing rust-lang#40303. This PR introduces the HirId type and generates a HirId for everything that would be assigned one (i.e. stuff in the HIR), but the HIR data types still use NodeId for now. Changing that is a big refactoring that I want to do in a separate PR.

A HirId uniquely identifies a node in the HIR of the current crate. It is composed of the owner, which is the DefIndex of the directly enclosing hir::Item, hir::TraitItem, or hir::ImplItem (i.e. the closest "item-like"), and the local_id which is unique within the given owner.

This PR is also running a number of consistency checks for the generated HirIds:

Does NodeId in the HIR have a corresponding HirId?
Is the owner part of each HirId consistent with its position in the HIR?
Do the numerical values of the local_id part all lie within a dense range of integers?

cc @rust-lang/compiler

r? @eddyb or @nikomatsakis

Correctly get source for metatdata-only crate type

Closes rust-lang#40535

However, I'm not sure how to approach writing a regression test since I'm still working on a reduced test case from the code that caused the ICE in the first place. It's not enough to have an unknown extern crate in a metadata crate, it depends on a few extra arguments but I'm not sure which yet.

Also replaced the unwrap() with a more informative expect().

r? @jseyfried

Update gcc used for dist-x86-linux builds

GCC 4.7 is too old to build LLVM 4.0, so this PR updates to 4.8.

r? @alexcrichton (I'll ping you again once travis is green and the test commit is removed).

…tsakis

Some preparations for directly computing the ICH of crate-metadata.

This PR contains some small fixes in preparation for direct metadata hashing. It mostly just moves stuff into places where it will be needed (making the module structure slightly cleaner along the way) and it fixes some omissions in the MIR region eraser.

r? @nikomatsakis

…, r=petrochenkov

Remove unused adt-def insertion by constructor DefIndex

It looks to me like ADT definitions weren't being looked up by constructor id, and a test run supports my theory.

In any case, I'm not sure it would have worked in its current configuration. If I understand correctly, the adt_def map entry from constructor id -> adt def would only be present after a successful call to queries::adt_def::get with the proper ADT DefIndex. Trying to look up an adt_def by the constructor index prior to a successful lookup by ADT index would fail since item.kind would be EntryKind::Fn (for the constructor function) and so would trigger the bug!.

r? @nikomatsakis

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @eddyb (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

📌 Commit 9307418 has been approved by frewsxcv

bors added a commit that referenced this pull request

Mar 23, 2017

This was referenced

Mar 23, 2017