about summary refs log tree commit diff
path: root/src/librustc_mir
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2016-11-17 17:31:01 -0800
committerGitHub <noreply@github.com>2016-11-17 17:31:01 -0800
commit35e8924dc5d5cc248f33767ff35f7aeb797a71ec (patch)
treeaa089b5c80ee7312c3b4f367288755334073049d /src/librustc_mir
parentc3565372c30257bccb127ee188a4d1f25f621db7 (diff)
parentc938007f90711d6acc8b55e15a5e3cf7cc147e91 (diff)
downloadrust-35e8924dc5d5cc248f33767ff35f7aeb797a71ec.tar.gz
rust-35e8924dc5d5cc248f33767ff35f7aeb797a71ec.zip
Auto merge of #37660 - nikomatsakis:incremental-36349, r=eddyb
Separate impl items from the parent impl

This change separates impl item bodies out of the impl itself. This gives incremental more resolution. In so doing, it refactors how the visitors work, and cleans up a bit of the collect/check logic (mostly by moving things out of collect that didn't really belong there, because they were just checking conditions).

However, this is not as effective as I expected, for a kind of frustrating reason. In particular, when invoking `foo.bar()` you still wind up with dependencies on private items. The problem is that the method resolution code scans that list for methods with the name `bar` -- and this winds up touching *all* the methods, even private ones.

I can imagine two obvious ways to fix this:

- separating fn bodies from fn sigs (#35078, currently being pursued by @flodiebold)
- a more aggressive model of incremental that @michaelwoerister has been advocating, in which we hash the intermediate results (e.g., the outputs of collect) so that we can see that the intermediate result hasn't changed, even if a particular impl item has changed.

So all in all I'm not quite sure whether to land this or not. =) It still seems like it has to be a win in some cases, but not with the test cases we have just now. I can try to gin up some test cases, but I'm not sure if they will be totally realistic. On the other hand, some of the early refactorings to the visitor trait seem worthwhile to me regardless.

cc #36349 -- well, this is basically a fix for that issue, I guess

r? @michaelwoerister

NB: Based atop of @eddyb's PR https://github.com/rust-lang/rust/pull/37402; don't land until that lands.
Diffstat (limited to 'src/librustc_mir')
-rw-r--r--src/librustc_mir/mir_map.rs4
-rw-r--r--src/librustc_mir/transform/qualify_consts.rs8
2 files changed, 8 insertions, 4 deletions
diff --git a/src/librustc_mir/mir_map.rs b/src/librustc_mir/mir_map.rs
index 4a50585efe3..992c0e9b5fc 100644
--- a/src/librustc_mir/mir_map.rs
+++ b/src/librustc_mir/mir_map.rs
@@ -38,9 +38,9 @@ use syntax_pos::Span;
 use std::mem;
 
 pub fn build_mir_for_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) {
-    tcx.visit_all_items_in_krate(DepNode::Mir, &mut BuildMir {
+    tcx.visit_all_item_likes_in_krate(DepNode::Mir, &mut BuildMir {
         tcx: tcx
-    });
+    }.as_deep_visitor());
 }
 
 /// A pass to lift all the types and substitutions in a Mir
diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs
index b33a7060e37..4ff2beb3fdb 100644
--- a/src/librustc_mir/transform/qualify_consts.rs
+++ b/src/librustc_mir/transform/qualify_consts.rs
@@ -277,8 +277,12 @@ impl<'a, 'tcx> Qualifier<'a, 'tcx, 'tcx> {
                             .and_then(|impl_node_id| self.tcx.map.find(impl_node_id))
                             .map(|node| {
                                 if let hir_map::NodeItem(item) = node {
-                                    if let hir::ItemImpl(_, _, _, _, _, ref methods) = item.node {
-                                        span = methods.first().map(|method| method.span);
+                                    if let hir::ItemImpl(.., ref impl_item_refs) = item.node {
+                                        span = impl_item_refs.first()
+                                                             .map(|iiref| {
+                                                                 self.tcx.map.impl_item(iiref.id)
+                                                                             .span
+                                                             });
                                     }
                                 }
                             });