convert the new conflicts to a soft error

Author: arielb1

src/librustc/lint/builtin.rs

@@ -198,6 +198,12 @@ declare_lint! {
     "detects generic lifetime arguments in path segments with late bound lifetime parameters"
 }
 
+declare_lint! {
+    pub INCOHERENT_FUNDAMENTAL_IMPLS,
+    Deny,
+    "potentially-conflicting impls were erroneously allowed"
+}
+
 declare_lint! {
     pub DEPRECATED,
     Warn,
@@ -254,6 +260,7 @@ impl LintPass for HardwiredLints {
             MISSING_FRAGMENT_SPECIFIER,
             PARENTHESIZED_PARAMS_IN_TYPES_AND_MODULES,
             LATE_BOUND_LIFETIME_ARGUMENTS,
+            INCOHERENT_FUNDAMENTAL_IMPLS,
             DEPRECATED,
             UNUSED_UNSAFE,
             UNUSED_MUT

src/librustc/traits/coherence.rs

@@ -13,6 +13,7 @@
 use hir::def_id::{DefId, LOCAL_CRATE};
 use syntax_pos::DUMMY_SP;
 use traits::{self, Normalized, SelectionContext, Obligation, ObligationCause, Reveal};
+use traits::IntercrateMode;
 use traits::select::IntercrateAmbiguityCause;
 use ty::{self, Ty, TyCtxt};
 use ty::subst::Subst;
@@ -42,16 +43,19 @@ pub struct OverlapResult<'tcx> {
 /// `ImplHeader` with those types substituted
 pub fn overlapping_impls<'cx, 'gcx, 'tcx>(infcx: &InferCtxt<'cx, 'gcx, 'tcx>,
                                           impl1_def_id: DefId,
-                                          impl2_def_id: DefId)
+                                          impl2_def_id: DefId,
+                                          intercrate_mode: IntercrateMode)
                                           -> Option<OverlapResult<'tcx>>
 {
     debug!("impl_can_satisfy(\
            impl1_def_id={:?}, \
-           impl2_def_id={:?})",
+           impl2_def_id={:?},
+           intercrate_mode={:?})",
            impl1_def_id,
-           impl2_def_id);
+           impl2_def_id,
+           intercrate_mode);
 
-    let selcx = &mut SelectionContext::intercrate(infcx);
+    let selcx = &mut SelectionContext::intercrate(infcx, intercrate_mode);
     overlap(selcx, impl1_def_id, impl2_def_id)
 }
 

src/librustc/traits/mod.rs

@@ -60,7 +60,8 @@ mod structural_impls;
 pub mod trans;
 mod util;
 
-#[derive(Copy, Clone, Debug)]
+// Whether to enable bug compatibility with issue #43355
+#[derive(Copy, Clone, PartialEq, Eq, Debug)]
 pub enum IntercrateMode {
     Issue43355,
     Fixed

src/librustc/traits/select.rs

@@ -15,6 +15,7 @@ use self::EvaluationResult::*;
 
 use super::coherence::{self, Conflict};
 use super::DerivedObligationCause;
+use super::IntercrateMode;
 use super::project;
 use super::project::{normalize_with_depth, Normalized, ProjectionCacheKey};
 use super::{PredicateObligation, TraitObligation, ObligationCause};
@@ -87,7 +88,7 @@ pub struct SelectionContext<'cx, 'gcx: 'cx+'tcx, 'tcx: 'cx> {
     /// other words, we consider `$0 : Bar` to be unimplemented if
     /// there is no type that the user could *actually name* that
     /// would satisfy it. This avoids crippling inference, basically.
-    intercrate: bool,
+    intercrate: Option<IntercrateMode>,
 
     inferred_obligations: SnapshotVec<InferredObligationsSnapshotVecDelegate<'tcx>>,
 
@@ -111,21 +112,24 @@ impl IntercrateAmbiguityCause {
     /// See #23980 for details.
     pub fn add_intercrate_ambiguity_hint<'a, 'tcx>(&self,
                                                    err: &mut ::errors::DiagnosticBuilder) {
+        err.note(&self.intercrate_ambiguity_hint());
+    }
+
+    pub fn intercrate_ambiguity_hint(&self) -> String {
         match self {
             &IntercrateAmbiguityCause::DownstreamCrate { ref trait_desc, ref self_desc } => {
                 let self_desc = if let &Some(ref ty) = self_desc {
                     format!(" for type `{}`", ty)
                 } else { "".to_string() };
-                err.note(&format!("downstream crates may implement trait `{}`{}",
-                                  trait_desc, self_desc));
+                format!("downstream crates may implement trait `{}`{}", trait_desc, self_desc)
             }
             &IntercrateAmbiguityCause::UpstreamCrateUpdate { ref trait_desc, ref self_desc } => {
                 let self_desc = if let &Some(ref ty) = self_desc {
                     format!(" for type `{}`", ty)
                 } else { "".to_string() };
-                err.note(&format!("upstream crates may add new impl of trait `{}`{} \
-                                  in future versions",
-                                  trait_desc, self_desc));
+                format!("upstream crates may add new impl of trait `{}`{} \
+                         in future versions",
+                        trait_desc, self_desc)
             }
         }
     }
@@ -417,17 +421,19 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
         SelectionContext {
             infcx,
             freshener: infcx.freshener(),
-            intercrate: false,
+            intercrate: None,
             inferred_obligations: SnapshotVec::new(),
             intercrate_ambiguity_causes: Vec::new(),
         }
     }
 
-    pub fn intercrate(infcx: &'cx InferCtxt<'cx, 'gcx, 'tcx>) -> SelectionContext<'cx, 'gcx, 'tcx> {
+    pub fn intercrate(infcx: &'cx InferCtxt<'cx, 'gcx, 'tcx>,
+                      mode: IntercrateMode) -> SelectionContext<'cx, 'gcx, 'tcx> {
+        debug!("intercrate({:?})", mode);
         SelectionContext {
             infcx,
             freshener: infcx.freshener(),
-            intercrate: true,
+            intercrate: Some(mode),
             inferred_obligations: SnapshotVec::new(),
             intercrate_ambiguity_causes: Vec::new(),
         }
@@ -758,7 +764,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
         debug!("evaluate_trait_predicate_recursively({:?})",
                obligation);
 
-        if !self.intercrate && obligation.is_global() {
+        if !self.intercrate.is_some() && obligation.is_global() {
             // If a param env is consistent, global obligations do not depend on its particular
             // value in order to work, so we can clear out the param env and get better
             // caching. (If the current param env is inconsistent, we don't care what happens).
@@ -814,7 +820,11 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
         // terms of `Fn` etc, but we could probably make this more
         // precise still.
         let unbound_input_types = stack.fresh_trait_ref.input_types().any(|ty| ty.is_fresh());
-        if unbound_input_types && self.intercrate && false {
+        // this check was an imperfect workaround for a bug n the old
+        // intercrate mode, it should be removed when that goes away.
+        if unbound_input_types &&
+            self.intercrate == Some(IntercrateMode::Issue43355)
+        {
             debug!("evaluate_stack({:?}) --> unbound argument, intercrate -->  ambiguous",
                    stack.fresh_trait_ref);
             // Heuristics: show the diagnostics when there are no candidates in crate.
@@ -1212,9 +1222,9 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
                        stack: &TraitObligationStack<'o, 'tcx>)
                        -> Option<Conflict>
     {
-        debug!("is_knowable(intercrate={})", self.intercrate);
+        debug!("is_knowable(intercrate={:?})", self.intercrate);
 
-        if !self.intercrate {
+        if !self.intercrate.is_some() {
             return None;
         }
 
@@ -1226,7 +1236,14 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
         // bound regions
         let trait_ref = predicate.skip_binder().trait_ref;
 
-        coherence::trait_ref_is_knowable(self.tcx(), trait_ref)
+        let result = coherence::trait_ref_is_knowable(self.tcx(), trait_ref);
+        if let (Some(Conflict::Downstream { used_to_be_broken: true }),
+                Some(IntercrateMode::Issue43355)) = (result, self.intercrate) {
+            debug!("is_knowable: IGNORING conflict to be bug-compatible with #43355");
+            None
+        } else {
+            result
+        }
     }
 
     /// Returns true if the global caches can be used.
@@ -1251,7 +1268,7 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
         // the master cache. Since coherence executes pretty quickly,
         // it's not worth going to more trouble to increase the
         // hit-rate I don't think.
-        if self.intercrate {
+        if self.intercrate.is_some() {
             return false;
         }
 

src/librustc/traits/specialize/mod.rs

@@ -30,6 +30,8 @@ use ty::{self, TyCtxt, TypeFoldable};
 use syntax_pos::DUMMY_SP;
 use std::rc::Rc;
 
+use lint;
+
 pub mod specialization_graph;
 
 /// Information pertinent to an overlapping impl error.
@@ -314,16 +316,33 @@ pub(super) fn specialization_graph_provider<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx
             // This is where impl overlap checking happens:
             let insert_result = sg.insert(tcx, impl_def_id);
             // Report error if there was one.
-            if let Err(overlap) = insert_result {
-                let mut err = struct_span_err!(tcx.sess,
-                                               tcx.span_of_impl(impl_def_id).unwrap(),
-                                               E0119,
-                                               "conflicting implementations of trait `{}`{}:",
-                                               overlap.trait_desc,
-                                               overlap.self_desc.clone().map_or(String::new(),
-                                                                                |ty| {
-                    format!(" for type `{}`", ty)
-                }));
+            let (overlap, used_to_be_allowed) = match insert_result {
+                Err(overlap) => (Some(overlap), false),
+                Ok(opt_overlap) => (opt_overlap, true)
+            };
+
+            if let Some(overlap) = overlap {
+                let msg = format!("conflicting implementations of trait `{}`{}:{}",
+                    overlap.trait_desc,
+                    overlap.self_desc.clone().map_or(
+                        String::new(), |ty| {
+                            format!(" for type `{}`", ty)
+                        }),
+                    if used_to_be_allowed { " (E0119)" } else { "" }
+                );
+                let mut err = if used_to_be_allowed {
+                    tcx.struct_span_lint_node(
+                        lint::builtin::INCOHERENT_FUNDAMENTAL_IMPLS,
+                        tcx.hir.as_local_node_id(impl_def_id).unwrap(),
+                        tcx.span_of_impl(impl_def_id).unwrap(),
+                        &msg)
+                } else {
+                    struct_span_err!(tcx.sess,
+                                     tcx.span_of_impl(impl_def_id).unwrap(),
+                                     E0119,
+                                     "{}",
+                                     msg)
+                };
 
                 match tcx.span_of_impl(overlap.with_impl) {
                     Ok(span) => {

src/librustc/traits/specialize/specialization_graph.rs

@@ -68,7 +68,7 @@ struct Children {
 /// The result of attempting to insert an impl into a group of children.
 enum Inserted {
     /// The impl was inserted as a new child in this group of children.
-    BecameNewSibling,
+    BecameNewSibling(Option<OverlapError>),
 
     /// The impl replaced an existing impl that specializes it.
     Replaced(DefId),
@@ -105,17 +105,39 @@ impl<'a, 'gcx, 'tcx> Children {
               simplified_self: Option<SimplifiedType>)
               -> Result<Inserted, OverlapError>
     {
+        let mut last_lint = None;
+
         for slot in match simplified_self {
             Some(sty) => self.filtered_mut(sty),
             None => self.iter_mut(),
         } {
             let possible_sibling = *slot;
 
+            let overlap_error = |overlap: traits::coherence::OverlapResult| {
+                // overlap, but no specialization; error out
+                let trait_ref = overlap.impl_header.trait_ref.unwrap();
+                let self_ty = trait_ref.self_ty();
+                OverlapError {
+                    with_impl: possible_sibling,
+                    trait_desc: trait_ref.to_string(),
+                    // only report the Self type if it has at least
+                    // some outer concrete shell; otherwise, it's
+                    // not adding much information.
+                    self_desc: if self_ty.has_concrete_skeleton() {
+                        Some(self_ty.to_string())
+                    } else {
+                        None
+                    },
+                    intercrate_ambiguity_causes: overlap.intercrate_ambiguity_causes,
+                }
+            };
+
             let tcx = tcx.global_tcx();
             let (le, ge) = tcx.infer_ctxt().enter(|infcx| {
                 let overlap = traits::overlapping_impls(&infcx,
                                                         possible_sibling,
-                                                        impl_def_id);
+                                                        impl_def_id,
+                                                        traits::IntercrateMode::Issue43355);
                 if let Some(overlap) = overlap {
                     if tcx.impls_are_allowed_to_overlap(impl_def_id, possible_sibling) {
                         return Ok((false, false));
@@ -125,22 +147,7 @@ impl<'a, 'gcx, 'tcx> Children {
                     let ge = tcx.specializes((possible_sibling, impl_def_id));
 
                     if le == ge {
-                        // overlap, but no specialization; error out
-                        let trait_ref = overlap.impl_header.trait_ref.unwrap();
-                        let self_ty = trait_ref.self_ty();
-                        Err(OverlapError {
-                            with_impl: possible_sibling,
-                            trait_desc: trait_ref.to_string(),
-                            // only report the Self type if it has at least
-                            // some outer concrete shell; otherwise, it's
-                            // not adding much information.
-                            self_desc: if self_ty.has_concrete_skeleton() {
-                                Some(self_ty.to_string())
-                            } else {
-                                None
-                            },
-                            intercrate_ambiguity_causes: overlap.intercrate_ambiguity_causes,
-                        })
+                        Err(overlap_error(overlap))
                     } else {
                         Ok((le, ge))
                     }
@@ -163,14 +170,27 @@ impl<'a, 'gcx, 'tcx> Children {
                     *slot = impl_def_id;
                 return Ok(Inserted::Replaced(possible_sibling));
             } else {
+                if !tcx.impls_are_allowed_to_overlap(impl_def_id, possible_sibling) {
+                    tcx.infer_ctxt().enter(|infcx| {
+                        if let Some(overlap) = traits::overlapping_impls(
+                            &infcx,
+                            possible_sibling,
+                            impl_def_id,
+                            traits::IntercrateMode::Fixed)
+                        {
+                            last_lint = Some(overlap_error(overlap));
+                        }
+                    });
+                }
+
                 // no overlap (error bailed already via ?)
             }
         }
 
         // no overlap with any potential siblings, so add as a new sibling
         debug!("placing as new sibling");
         self.insert_blindly(tcx, impl_def_id);
-        Ok(Inserted::BecameNewSibling)
+        Ok(Inserted::BecameNewSibling(last_lint))
     }
 
     fn iter_mut(&'a mut self) -> Box<Iterator<Item = &'a mut DefId> + 'a> {
@@ -199,7 +219,7 @@ impl<'a, 'gcx, 'tcx> Graph {
     pub fn insert(&mut self,
                   tcx: TyCtxt<'a, 'gcx, 'tcx>,
                   impl_def_id: DefId)
-                  -> Result<(), OverlapError> {
+                  -> Result<Option<OverlapError>, OverlapError> {
         assert!(impl_def_id.is_local());
 
         let trait_ref = tcx.impl_trait_ref(impl_def_id).unwrap();
@@ -220,10 +240,11 @@ impl<'a, 'gcx, 'tcx> Graph {
             self.parent.insert(impl_def_id, trait_def_id);
             self.children.entry(trait_def_id).or_insert(Children::new())
                 .insert_blindly(tcx, impl_def_id);
-            return Ok(());
+            return Ok(None);
         }
 
         let mut parent = trait_def_id;
+        let mut last_lint = None;
         let simplified = fast_reject::simplify_type(tcx, trait_ref.self_ty(), false);
 
         // Descend the specialization tree, where `parent` is the current parent node
@@ -234,7 +255,8 @@ impl<'a, 'gcx, 'tcx> Graph {
                 .insert(tcx, impl_def_id, simplified)?;
 
             match insert_result {
-                BecameNewSibling => {
+                BecameNewSibling(opt_lint) => {
+                    last_lint = opt_lint;
                     break;
                 }
                 Replaced(new_child) => {
@@ -251,7 +273,7 @@ impl<'a, 'gcx, 'tcx> Graph {
         }
 
         self.parent.insert(impl_def_id, parent);
-        Ok(())
+        Ok(last_lint)
     }
 
     /// Insert cached metadata mapping from a child impl back to its parent.

src/librustc_lint/lib.rs

@@ -243,6 +243,10 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
             id: LintId::of(LATE_BOUND_LIFETIME_ARGUMENTS),
             reference: "issue #42868 <https://github.com/rust-lang/rust/issues/42868>",
         },
+        FutureIncompatibleInfo {
+            id: LintId::of(INCOHERENT_FUNDAMENTAL_IMPLS),
+            reference: "issue #46205 <https://github.com/rust-lang/rust/issues/46205>",
+        },
         ]);
 
     // Register renamed and removed lints

src/librustc_typeck/coherence/inherent_impls_overlap.rs

@@ -12,9 +12,11 @@ use namespace::Namespace;
 use rustc::hir::def_id::{CrateNum, DefId, LOCAL_CRATE};
 use rustc::hir;
 use rustc::hir::itemlikevisit::ItemLikeVisitor;
-use rustc::traits;
+use rustc::traits::{self, IntercrateMode};
 use rustc::ty::TyCtxt;
 
+use lint;
+
 pub fn crate_inherent_impls_overlap_check<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
                                                     crate_num: CrateNum) {
     assert_eq!(crate_num, LOCAL_CRATE);
@@ -28,7 +30,8 @@ struct InherentOverlapChecker<'a, 'tcx: 'a> {
 
 impl<'a, 'tcx> InherentOverlapChecker<'a, 'tcx> {
     fn check_for_common_items_in_impls(&self, impl1: DefId, impl2: DefId,
-                                       overlap: traits::OverlapResult) {
+                                       overlap: traits::OverlapResult,
+                                       used_to_be_allowed: bool) {
 
         let name_and_namespace = |def_id| {
             let item = self.tcx.associated_item(def_id);
@@ -43,11 +46,21 @@ impl<'a, 'tcx> InherentOverlapChecker<'a, 'tcx> {
 
             for &item2 in &impl_items2[..] {
                 if (name, namespace) == name_and_namespace(item2) {
-                    let mut err = struct_span_err!(self.tcx.sess,
-                                                   self.tcx.span_of_impl(item1).unwrap(),
-                                                   E0592,
-                                                   "duplicate definitions with name `{}`",
-                                                   name);
+                    let node_id = self.tcx.hir.as_local_node_id(impl1);
+                    let mut err = if used_to_be_allowed && node_id.is_some() {
+                        self.tcx.struct_span_lint_node(
+                            lint::builtin::INCOHERENT_FUNDAMENTAL_IMPLS,
+                            node_id.unwrap(),
+                            self.tcx.span_of_impl(item1).unwrap(),
+                            &format!("duplicate definitions with name `{}` (E0592)", name)
+                        )
+                    } else {
+                        struct_span_err!(self.tcx.sess,
+                                         self.tcx.span_of_impl(item1).unwrap(),
+                                         E0592,
+                                         "duplicate definitions with name `{}`",
+                                         name)
+                    };
 
                     err.span_label(self.tcx.span_of_impl(item1).unwrap(),
                                    format!("duplicate definitions for `{}`", name));
@@ -69,12 +82,30 @@ impl<'a, 'tcx> InherentOverlapChecker<'a, 'tcx> {
 
         for (i, &impl1_def_id) in impls.iter().enumerate() {
             for &impl2_def_id in &impls[(i + 1)..] {
-                self.tcx.infer_ctxt().enter(|infcx| {
+                let used_to_be_allowed = self.tcx.infer_ctxt().enter(|infcx| {
                     if let Some(overlap) =
-                            traits::overlapping_impls(&infcx, impl1_def_id, impl2_def_id) {
-                        self.check_for_common_items_in_impls(impl1_def_id, impl2_def_id, overlap)
+                        traits::overlapping_impls(&infcx, impl1_def_id, impl2_def_id,
+                                                  IntercrateMode::Issue43355)
+                    {
+                        self.check_for_common_items_in_impls(
+                            impl1_def_id, impl2_def_id, overlap, false);
+                        false
+                    } else {
+                        true
                     }
                 });
+
+                if used_to_be_allowed {
+                    self.tcx.infer_ctxt().enter(|infcx| {
+                        if let Some(overlap) =
+                            traits::overlapping_impls(&infcx, impl1_def_id, impl2_def_id,
+                                                      IntercrateMode::Fixed)
+                        {
+                            self.check_for_common_items_in_impls(
+                                impl1_def_id, impl2_def_id, overlap, true);
+                        }
+                    });
+                }
             }
         }
     }
@@ -100,4 +131,3 @@ impl<'a, 'tcx, 'v> ItemLikeVisitor<'v> for InherentOverlapChecker<'a, 'tcx> {
     fn visit_impl_item(&mut self, _impl_item: &hir::ImplItem) {
     }
 }
-

src/test/compile-fail/issue-43355.rs

@@ -22,6 +22,7 @@ impl<X, T> Trait1<X> for T where T: Trait2<X> {
 
 impl<X> Trait1<Box<X>> for A {
 //~^ ERROR conflicting implementations of trait
+//~| hard error
 //~| downstream crates may implement trait `Trait2<std::boxed::Box<_>>` for type `A`
     type Output = i32;
 }

src/test/run-pass/issue-43355.rs

@@ -0,0 +1,38 @@
+// Copyright 2017 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.
+
+// Check that the code for issue #43355 can run without an ICE, please remove
+// this test when it becomes an hard error.
+
+#![allow(incoherent_fundamental_impls)]
+
+pub trait Trait1<X> {
+    type Output;
+}
+pub trait Trait2<X> {}
+
+impl<X, T> Trait1<X> for T where T: Trait2<X> {
+    type Output = ();
+}
+impl<X> Trait1<Box<X>> for A {
+    type Output = i32;
+}
+
+pub struct A;
+
+fn f<X, T: Trait1<Box<X>>>() {
+    println!("k: {}", ::std::mem::size_of::<<T as Trait1<Box<X>>>::Output>());
+}
+
+pub fn g<X, T: Trait2<Box<X>>>() {
+    f::<X, T>();
+}
+
+fn main() {}