Implemented more sane checks for breaking trait items. Fixes rust-lan…

…g#20.
eddyb · Jul 20, 2017 · c64a89b · c64a89b
1 parent 6fc7023
commit c64a89b
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 2 deletions.
diff --git a/src/semcheck/changes.rs b/src/semcheck/changes.rs
@@ -322,6 +322,38 @@ impl<'tcx> Change<'tcx> {
         self.changes.push((type_, span));
     }
 
+    /// Check whether a trait item contains breaking changes preventing further analysis of it's
+    /// child items.
+    fn trait_item_breaking(&self) -> bool {
+        for change in &self.changes {
+            match change.0 {
+                ItemMadePrivate |
+                KindDifference |
+                RegionParameterRemoved |
+                TypeParameterRemoved { .. } |
+                VariantAdded |
+                VariantRemoved |
+                VariantFieldAdded { .. } |
+                VariantFieldRemoved { .. } |
+                VariantStyleChanged { .. } |
+                TypeChanged { .. } |
+                FnConstChanged { now_const: false } |
+                MethodSelfChanged { now_self: false } |
+                Unknown => return true,
+                RegionParameterAdded |
+                MethodSelfChanged { now_self: true } |
+                TraitItemAdded { .. } |
+                TraitItemRemoved { .. } |
+                ItemMadePublic |
+                TypeParameterAdded { .. } |
+                TraitUnsafetyChanged { .. } |
+                FnConstChanged { now_const: true } => (),
+            }
+        }
+
+        false
+    }
+
     /// Get the change's category.
     fn to_category(&self) -> ChangeCategory {
         self.max.clone()
@@ -464,7 +496,16 @@ impl<'tcx> ChangeSet<'tcx> {
         // we only care about items that were present in both versions.
         self.changes
             .get(&old)
-            .map(|changes| changes.to_category() == Breaking)
+            .map(|change| change.to_category() == Breaking)
+            .unwrap_or(false)
+    }
+
+    /// Check whether a trait item contains breaking changes preventing further analysis of it's
+    /// child items.
+    pub fn trait_item_breaking(&self, old: DefId) -> bool {
+        self.changes
+            .get(&old)
+            .map(|change| change.trait_item_breaking())
             .unwrap_or(false)
     }
 

diff --git a/src/semcheck/traverse.rs b/src/semcheck/traverse.rs
@@ -543,7 +543,7 @@ fn diff_types<'a, 'tcx>(changes: &mut ChangeSet<'tcx>,
 
     if changes.item_breaking(old_def_id) ||
             id_mapping.get_trait_def(&old_def_id)
-                .map_or(false, |did| changes.item_breaking(did)) {
+                .map_or(false, |did| changes.trait_item_breaking(did)) {
         return;
     }
 

diff --git a/tests/cases/traits/stdout b/tests/cases/traits/stdout
@@ -36,6 +36,14 @@ note: added defaulted item to trait (technically breaking)
 10 | |     }
    | |_____^
 
+warning: breaking changes in `test7`
+  --> $REPO_PATH/tests/cases/traits/new.rs:11:5
+   |
+11 |     fn test7() -> u16;
+   |     ^^^^^^^^^^^^^^^^^^
+   |
+   = warning: type error: expected u8, found u16 (breaking)
+
 warning: breaking changes in `test8`
   --> $REPO_PATH/tests/cases/traits/new.rs:12:5
    |