Auto merge of #54906 - qnighy:fix-issue-50452, r=nikomatsakis

Reattach all grandchildren when constructing specialization graph.

Specialization graphs are constructed by incrementally adding impls in the order of declaration. If the impl being added has its specializations in the graph already, they should be reattached under the impl. However, the current implementation only reattaches the one found first. Therefore, in the following specialization graph,

```
  Tr1
   |
   I3
  /  \
 I1  I2
```

If `I1`, `I2`, and `I3` are declared in this order, the compiler mistakenly constructs the following graph:

```
  Tr1
  /  \
 I3  I2
  |
 I1
```

This patch fixes the reattach procedure to include all specializing grandchildren-to-be.

Fixes #50452.

Author: bors

Diff for: src/librustc/traits/specialize/specialization_graph.rs

@@ -73,8 +73,8 @@ enum Inserted {
     /// The impl was inserted as a new child in this group of children.
     BecameNewSibling(Option<OverlapError>),
 
-    /// The impl should replace an existing impl X, because the impl specializes X.
-    ReplaceChild(DefId),
+    /// The impl should replace existing impls [X1, ..], because the impl specializes X1, X2, etc.
+    ReplaceChildren(Vec<DefId>),
 
     /// The impl is a specialization of an existing child.
     ShouldRecurseOn(DefId),
@@ -124,6 +124,7 @@ impl<'a, 'gcx, 'tcx> Children {
               -> Result<Inserted, OverlapError>
     {
         let mut last_lint = None;
+        let mut replace_children = Vec::new();
 
         debug!(
             "insert(impl_def_id={:?}, simplified_self={:?})",
@@ -194,7 +195,7 @@ impl<'a, 'gcx, 'tcx> Children {
                 debug!("placing as parent of TraitRef {:?}",
                        tcx.impl_trait_ref(possible_sibling).unwrap());
 
-                return Ok(Inserted::ReplaceChild(possible_sibling));
+                replace_children.push(possible_sibling);
             } else {
                 if !tcx.impls_are_allowed_to_overlap(impl_def_id, possible_sibling) {
                     traits::overlapping_impls(
@@ -211,6 +212,10 @@ impl<'a, 'gcx, 'tcx> Children {
             }
         }
 
+        if !replace_children.is_empty() {
+            return Ok(Inserted::ReplaceChildren(replace_children));
+        }
+
         // no overlap with any potential siblings, so add as a new sibling
         debug!("placing as new sibling");
         self.insert_blindly(tcx, impl_def_id);
@@ -282,7 +287,7 @@ impl<'a, 'gcx, 'tcx> Graph {
                     last_lint = opt_lint;
                     break;
                 }
-                ReplaceChild(grand_child_to_be) => {
+                ReplaceChildren(grand_children_to_be) => {
                     // We currently have
                     //
                     //     P
@@ -302,17 +307,23 @@ impl<'a, 'gcx, 'tcx> Graph {
                         let siblings = self.children
                             .get_mut(&parent)
                             .unwrap();
-                        siblings.remove_existing(tcx, grand_child_to_be);
+                        for &grand_child_to_be in &grand_children_to_be {
+                            siblings.remove_existing(tcx, grand_child_to_be);
+                        }
                         siblings.insert_blindly(tcx, impl_def_id);
                     }
 
                     // Set G's parent to N and N's parent to P
-                    self.parent.insert(grand_child_to_be, impl_def_id);
+                    for &grand_child_to_be in &grand_children_to_be {
+                        self.parent.insert(grand_child_to_be, impl_def_id);
+                    }
                     self.parent.insert(impl_def_id, parent);
 
                     // Add G as N's child.
-                    self.children.entry(impl_def_id).or_default()
-                        .insert_blindly(tcx, grand_child_to_be);
+                    for &grand_child_to_be in &grand_children_to_be {
+                        self.children.entry(impl_def_id).or_default()
+                            .insert_blindly(tcx, grand_child_to_be);
+                    }
                     break;
                 }
                 ShouldRecurseOn(new_parent) => {

Diff for: src/test/compile-fail/specialization/issue-50452.rs

@@ -0,0 +1,32 @@
+// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// compile-fail
+
+#![feature(specialization)]
+
+pub trait Foo {
+    fn foo();
+}
+
+impl Foo for i32 {}
+impl Foo for i64 {
+    fn foo() {}
+    //~^ERROR `foo` specializes an item from a parent `impl`
+}
+impl<T> Foo for T {
+    fn foo() {}
+}
+
+fn main() {
+    i32::foo();
+    i64::foo();
+    u8::foo();
+}

Diff for: src/test/run-pass/specialization/issue-50452.rs

@@ -0,0 +1,29 @@
+// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// run-pass
+
+#![feature(specialization)]
+
+pub trait Foo {
+    fn foo();
+}
+
+impl Foo for i32 {}
+impl Foo for i64 {}
+impl<T> Foo for T {
+    fn foo() {}
+}
+
+fn main() {
+    i32::foo();
+    i64::foo();
+    u8::foo();
+}