A delayed path removal processing scheme. Fixes rust-lang#19.

eddyb · Jul 23, 2017 · 59e9b18 · 59e9b18
1 parent 63377fc
commit 59e9b18
Show file tree

Hide file tree

Showing 3 changed files with 33 additions and 19 deletions.
diff --git a/src/semcheck/traverse.rs b/src/semcheck/traverse.rs
@@ -74,6 +74,10 @@ fn diff_structure<'a, 'tcx>(changes: &mut ChangeSet,
     let mut visited = HashSet::new();
     let mut children = NameMapping::default();
     let mut mod_queue = VecDeque::new();
+    // Removals are processed with a delay to avoid creating multiple path change entries.
+    // This is necessary, since the order in which added or removed paths are found wrt each
+    // other and their item's definition can't be relied upon.
+    let mut removals = Vec::new();
 
     mod_queue.push_back((old, new, Public, Public));
 
@@ -222,10 +226,11 @@ fn diff_structure<'a, 'tcx>(changes: &mut ChangeSet,
                         continue;
                     }
 
-                    if old_vis == Public && cstore.visibility(o.def.def_id()) == Public {
-                        let o_did = o.def.def_id();
-                        changes.new_path(o_did, o.ident.name, tcx.def_span(o_did));
-                        changes.add_path_removal(o_did, o.span);
+                    let o_did = o.def.def_id();
+
+                    if old_vis == Public && cstore.visibility(o_did) == Public {
+                        // delay the handling of removals until the id mapping is complete
+                        removals.push(o);
                     }
                 }
                 (None, Some(n)) => {
@@ -234,8 +239,9 @@ fn diff_structure<'a, 'tcx>(changes: &mut ChangeSet,
                         continue;
                     }
 
-                    if new_vis == Public && cstore.visibility(n.def.def_id()) == Public {
-                        let n_did = n.def.def_id();
+                    let n_did = n.def.def_id();
+
+                    if new_vis == Public && cstore.visibility(n_did) == Public {
                         changes.new_path(n_did, n.ident.name, tcx.def_span(n_did));
                         changes.add_path_addition(n_did, n.span);
                     }
@@ -244,6 +250,21 @@ fn diff_structure<'a, 'tcx>(changes: &mut ChangeSet,
             }
         }
     }
+
+    // finally, process item removals
+    for o in removals.drain(..) {
+        let o_did = o.def.def_id();
+
+        // reuse an already existing path change entry, if possible
+        if id_mapping.contains_id(o_did) {
+            let n_did = id_mapping.get_new_id(o_did);
+            changes.new_path(n_did, o.ident.name, tcx.def_span(n_did));
+            changes.add_path_removal(n_did, o.span);
+        } else {
+            changes.new_path(o_did, o.ident.name, tcx.def_span(o_did));
+            changes.add_path_removal(o_did, o.span);
+        }
+    }
 }
 
 /// Given two fn items, perform structural checks.

diff --git a/tests/cases/mix/stdout b/tests/cases/mix/stdout
@@ -1,16 +1,4 @@
 version bump: 1.0.0 -> (breaking) -> 2.0.0
-warning: path changes to `Abc`
- --> $REPO_PATH/tests/cases/mix/old.rs:2:5
-  |
-2 |     pub struct Abc;
-  |     ^^^^^^^^^^^^^^^
-  |
-warning: removed path (breaking)
- --> $REPO_PATH/tests/cases/mix/old.rs:10:9
-  |
-10| pub use self::a::Abc;
-  |         ^^^^^^^^^^^^
-
 warning: breaking changes in `Abc`
  --> $REPO_PATH/tests/cases/mix/new.rs:2:5
   |
@@ -33,6 +21,11 @@ warning: path changes to `Abc`
 2 |     pub enum Abc {}
   |     ^^^^^^^^^^^^^^^
   |
+warning: removed path (breaking)
+ --> $REPO_PATH/tests/cases/mix/old.rs:10:9
+  |
+10| pub use self::a::Abc;
+  |         ^^^^^^^^^^^^
 note: added path (technically breaking)
  --> $REPO_PATH/tests/cases/mix/new.rs:7:13
   |

diff --git a/tests/cases/removal_path/stdout b/tests/cases/removal_path/stdout
@@ -1,6 +1,6 @@
 version bump: 1.0.0 -> (breaking) -> 2.0.0
 warning: path changes to `Abc`
- --> $REPO_PATH/tests/cases/removal_path/old.rs:2:5
+ --> $REPO_PATH/tests/cases/removal_path/new.rs:2:5
   |
 2 |     pub struct Abc;
   |     ^^^^^^^^^^^^^^^