Auto merge of #31925 - aturon:inherent-overlap, r=nikomatsakis

Forbid items with the same name from appearing in overlapping inherent impl blocks

For example, the following is now correctly illegal:

```rust
struct Foo;

impl Foo {
    fn id() {}
}

impl Foo {
    fn id() {}
}
```

"Overlapping" here is determined the same way it is for traits (and in fact shares the same code path): roughly, there must be some way of substituting any generic types to unify the impls, such that none of the `where` clauses are provably unsatisfiable under such a unification.

Along the way, this PR also introduces an `ImplHeader` abstraction (the first commit) that makes it easier to work with impls abstractly (without caring whether they are trait or inherent impl blocks); see the first commit.

Closes #22889
r? @nikomatsakis

src/librustc/dep_graph/mod.rs

@@ -58,6 +58,7 @@ pub enum DepNode {
  CoherenceCheckImpl(DefId),
  CoherenceOverlapCheck(DefId),
  CoherenceOverlapCheckSpecial(DefId),
+ CoherenceOverlapInherentCheck(DefId),
  CoherenceOrphanCheck(DefId),
  Variance,
  WfCheck(DefId),

src/librustc/middle/infer/mod.rs

@@ -458,14 +458,13 @@ pub fn mk_eqty<'a, 'tcx>(cx: &InferCtxt<'a, 'tcx>,
 }
 
 pub fn mk_eq_trait_refs<'a, 'tcx>(cx: &InferCtxt<'a, 'tcx>,
-  a_is_expected: bool,
-  origin: TypeOrigin,
-  a: ty::TraitRef<'tcx>,
-  b: ty::TraitRef<'tcx>)
-  -> UnitResult<'tcx>
+ a_is_expected: bool,
+ origin: TypeOrigin,
+ a: ty::TraitRef<'tcx>,
+ b: ty::TraitRef<'tcx>)
+ -> UnitResult<'tcx>
 {
- debug!("mk_eq_trait_refs({:?} <: {:?})",
- a, b);
+ debug!("mk_eq_trait_refs({:?} = {:?})", a, b);
  cx.eq_trait_refs(a_is_expected, origin, a, b)
 }
 
@@ -476,11 +475,25 @@ pub fn mk_sub_poly_trait_refs<'a, 'tcx>(cx: &InferCtxt<'a, 'tcx>,
  b: ty::PolyTraitRef<'tcx>)
  -> UnitResult<'tcx>
 {
- debug!("mk_sub_poly_trait_refs({:?} <: {:?})",
- a, b);
+ debug!("mk_sub_poly_trait_refs({:?} <: {:?})", a, b);
  cx.sub_poly_trait_refs(a_is_expected, origin, a, b)
 }
 
+pub fn mk_eq_impl_headers<'a, 'tcx>(cx: &InferCtxt<'a, 'tcx>,
+ a_is_expected: bool,
+ origin: TypeOrigin,
+ a: &ty::ImplHeader<'tcx>,
+ b: &ty::ImplHeader<'tcx>)
+ -> UnitResult<'tcx>
+{
+ debug!("mk_eq_impl_header({:?} = {:?})", a, b);
+ match (a.trait_ref, b.trait_ref) {
+ (Some(a_ref), Some(b_ref)) => mk_eq_trait_refs(cx, a_is_expected, origin, a_ref, b_ref),
+ (None, None) => mk_eqty(cx, a_is_expected, origin, a.self_ty, b.self_ty),
+ _ => cx.tcx.sess.bug("mk_eq_impl_headers given mismatched impl kinds"),
+ }
+}
+
 fn expected_found<T>(a_is_expected: bool,
  a: T,
  b: T)

src/librustc/middle/traits/coherence.rs

@@ -10,29 +10,25 @@
 
 //! See `README.md` for high-level documentation
 
-use super::Normalized;
-use super::SelectionContext;
-use super::ObligationCause;
-use super::PredicateObligation;
-use super::project;
-use super::util;
+use super::{SelectionContext};
+use super::{Obligation, ObligationCause};
 
 use middle::cstore::LOCAL_CRATE;
 use middle::def_id::DefId;
-use middle::subst::{Subst, Substs, TypeSpace};
+use middle::subst::TypeSpace;
 use middle::ty::{self, Ty, TyCtxt};
 use middle::infer::{self, InferCtxt, TypeOrigin};
-use syntax::codemap::{DUMMY_SP, Span};
+use syntax::codemap::DUMMY_SP;
 
 #[derive(Copy, Clone)]
 struct InferIsLocal(bool);
 
-/// If there are types that satisfy both impls, returns a `TraitRef`
+/// If there are types that satisfy both impls, returns an `ImplTy`
 /// with those types substituted (by updating the given `infcx`)
 pub fn overlapping_impls<'cx, 'tcx>(infcx: &InferCtxt<'cx, 'tcx>,
  impl1_def_id: DefId,
  impl2_def_id: DefId)
- -> Option<ty::TraitRef<'tcx>>
+ -> Option<ty::ImplHeader<'tcx>>
 {
  debug!("impl_can_satisfy(\
  impl1_def_id={:?}, \
@@ -45,34 +41,28 @@ pub fn overlapping_impls<'cx, 'tcx>(infcx: &InferCtxt<'cx, 'tcx>,
 }
 
 /// Can both impl `a` and impl `b` be satisfied by a common type (including
-/// `where` clauses)? If so, returns a `TraitRef` that unifies the two impls.
+/// `where` clauses)? If so, returns an `ImplHeader` that unifies the two impls.
 fn overlap<'cx, 'tcx>(selcx: &mut SelectionContext<'cx, 'tcx>,
  a_def_id: DefId,
  b_def_id: DefId)
- -> Option<ty::TraitRef<'tcx>>
+ -> Option<ty::ImplHeader<'tcx>>
 {
  debug!("overlap(a_def_id={:?}, b_def_id={:?})",
  a_def_id,
  b_def_id);
 
- let (a_trait_ref, a_obligations) = impl_trait_ref_and_oblig(selcx,
- a_def_id,
- util::fresh_type_vars_for_impl);
+ let a_impl_header = ty::ImplHeader::with_fresh_ty_vars(selcx, a_def_id);
+ let b_impl_header = ty::ImplHeader::with_fresh_ty_vars(selcx, b_def_id);
 
- let (b_trait_ref, b_obligations) = impl_trait_ref_and_oblig(selcx,
- b_def_id,
- util::fresh_type_vars_for_impl);
-
- debug!("overlap: a_trait_ref={:?} a_obligations={:?}", a_trait_ref, a_obligations);
-
- debug!("overlap: b_trait_ref={:?} b_obligations={:?}", b_trait_ref, b_obligations);
+ debug!("overlap: a_impl_header={:?}", a_impl_header);
+ debug!("overlap: b_impl_header={:?}", b_impl_header);
 
  // Do `a` and `b` unify? If not, no overlap.
- if let Err(_) = infer::mk_eq_trait_refs(selcx.infcx(),
- true,
- TypeOrigin::Misc(DUMMY_SP),
- a_trait_ref,
- b_trait_ref) {
+ if let Err(_) = infer::mk_eq_impl_headers(selcx.infcx(),
+  true,
+  TypeOrigin::Misc(DUMMY_SP),
+  &a_impl_header,
+  &b_impl_header) {
  return None;
  }
 
@@ -81,17 +71,21 @@ fn overlap<'cx, 'tcx>(selcx: &mut SelectionContext<'cx, 'tcx>,
  // Are any of the obligations unsatisfiable? If so, no overlap.
  let infcx = selcx.infcx();
  let opt_failing_obligation =
- a_obligations.iter()
- .chain(&b_obligations)
- .map(|o| infcx.resolve_type_vars_if_possible(o))
+ a_impl_header.predicates
+ .iter()
+ .chain(&b_impl_header.predicates)
+ .map(|p| infcx.resolve_type_vars_if_possible(p))
+ .map(|p| Obligation { cause: ObligationCause::dummy(),
+ recursion_depth: 0,
+ predicate: p })
  .find(|o| !selcx.evaluate_obligation(o));
 
  if let Some(failing_obligation) = opt_failing_obligation {
  debug!("overlap: obligation unsatisfiable {:?}", failing_obligation);
  return None
  }
 
- Some(selcx.infcx().resolve_type_vars_if_possible(&a_trait_ref))
+ Some(selcx.infcx().resolve_type_vars_if_possible(&a_impl_header))
 }
 
 pub fn trait_ref_is_knowable<'tcx>(tcx: &TyCtxt<'tcx>, trait_ref: &ty::TraitRef<'tcx>) -> bool
@@ -125,44 +119,6 @@ pub fn trait_ref_is_knowable<'tcx>(tcx: &TyCtxt<'tcx>, trait_ref: &ty::TraitRef<
  orphan_check_trait_ref(tcx, trait_ref, InferIsLocal(true)).is_err()
 }
 
-type SubstsFn = for<'a,'tcx> fn(infcx: &InferCtxt<'a, 'tcx>,
- span: Span,
- impl_def_id: DefId)
- -> Substs<'tcx>;
-
-/// Instantiate fresh variables for all bound parameters of the impl
-/// and return the impl trait ref with those variables substituted.
-fn impl_trait_ref_and_oblig<'a,'tcx>(selcx: &mut SelectionContext<'a,'tcx>,
- impl_def_id: DefId,
- substs_fn: SubstsFn)
- -> (ty::TraitRef<'tcx>,
- Vec<PredicateObligation<'tcx>>)
-{
- let impl_substs =
- &substs_fn(selcx.infcx(), DUMMY_SP, impl_def_id);
- let impl_trait_ref =
- selcx.tcx().impl_trait_ref(impl_def_id).unwrap();
- let impl_trait_ref =
- impl_trait_ref.subst(selcx.tcx(), impl_substs);
- let Normalized { value: impl_trait_ref, obligations: normalization_obligations1 } =
- project::normalize(selcx, ObligationCause::dummy(), &impl_trait_ref);
-
- let predicates = selcx.tcx().lookup_predicates(impl_def_id);
- let predicates = predicates.instantiate(selcx.tcx(), impl_substs);
- let Normalized { value: predicates, obligations: normalization_obligations2 } =
- project::normalize(selcx, ObligationCause::dummy(), &predicates);
- let impl_obligations =
- util::predicates_for_generics(ObligationCause::dummy(), 0, &predicates);
-
- let impl_obligations: Vec<_> =
- impl_obligations.into_iter()
- .chain(normalization_obligations1)
- .chain(normalization_obligations2)
- .collect();
-
- (impl_trait_ref, impl_obligations)
-}
-
 pub enum OrphanCheckErr<'tcx> {
  NoLocalInputType,
  UncoveredTy(Ty<'tcx>),

src/librustc/middle/traits/select.rs

@@ -391,7 +391,6 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
  // The result is "true" if the obligation *may* hold and "false" if
  // we can be sure it does not.
 
-
  /// Evaluates whether the obligation `obligation` can be satisfied (by any means).
  pub fn evaluate_obligation(&mut self,
  obligation: &PredicateObligation<'tcx>)

src/librustc/middle/ty/fold.rs

@@ -146,6 +146,10 @@ pub trait TypeFolder<'tcx> : Sized {
  t.super_fold_with(self)
  }
 
+ fn fold_impl_header(&mut self, imp: &ty::ImplHeader<'tcx>) -> ty::ImplHeader<'tcx> {
+ imp.super_fold_with(self)
+ }
+
  fn fold_substs(&mut self,
  substs: &subst::Substs<'tcx>)
  -> subst::Substs<'tcx> {

src/librustc/middle/ty/mod.rs

@@ -152,6 +152,41 @@ impl ImplOrTraitItemContainer {
  }
 }
 
+/// The "header" of an impl is everything outside the body: a Self type, a trait
+/// ref (in the case of a trait impl), and a set of predicates (from the
+/// bounds/where clauses).
+#[derive(Clone, PartialEq, Eq, Hash, Debug)]
+pub struct ImplHeader<'tcx> {
+ pub impl_def_id: DefId,
+ pub self_ty: Ty<'tcx>,
+ pub trait_ref: Option<TraitRef<'tcx>>,
+ pub predicates: Vec<Predicate<'tcx>>,
+}
+
+impl<'tcx> ImplHeader<'tcx> {
+ pub fn with_fresh_ty_vars<'a>(selcx: &mut traits::SelectionContext<'a, 'tcx>,
+ impl_def_id: DefId)
+ -> ImplHeader<'tcx>
+ {
+ let tcx = selcx.tcx();
+ let impl_generics = tcx.lookup_item_type(impl_def_id).generics;
+ let impl_substs = selcx.infcx().fresh_substs_for_generics(DUMMY_SP, &impl_generics);
+
+ let header = ImplHeader {
+ impl_def_id: impl_def_id,
+ self_ty: tcx.lookup_item_type(impl_def_id).ty,
+ trait_ref: tcx.impl_trait_ref(impl_def_id),
+ predicates: tcx.lookup_predicates(impl_def_id).predicates.into_vec(),
+ }.subst(tcx, &impl_substs);
+
+ let traits::Normalized { value: mut header, obligations } =
+ traits::normalize(selcx, traits::ObligationCause::dummy(), &header);
+
+ header.predicates.extend(obligations.into_iter().map(|o| o.predicate));
+ header
+ }
+}
+
 #[derive(Clone)]
 pub enum ImplOrTraitItem<'tcx> {
  ConstTraitItem(Rc<AssociatedConst<'tcx>>),

src/librustc/middle/ty/structural_impls.rs

@@ -446,6 +446,27 @@ impl<'tcx> TypeFoldable<'tcx> for ty::TraitRef<'tcx> {
  }
 }
 
+impl<'tcx> TypeFoldable<'tcx> for ty::ImplHeader<'tcx> {
+ fn super_fold_with<F: TypeFolder<'tcx>>(&self, folder: &mut F) -> Self {
+ ty::ImplHeader {
+ impl_def_id: self.impl_def_id,
+ self_ty: self.self_ty.fold_with(folder),
+ trait_ref: self.trait_ref.map(|t| t.fold_with(folder)),
+ predicates: self.predicates.iter().map(|p| p.fold_with(folder)).collect(),
+ }
+ }
+
+ fn fold_with<F: TypeFolder<'tcx>>(&self, folder: &mut F) -> Self {
+ folder.fold_impl_header(self)
+ }
+
+ fn super_visit_with<V: TypeVisitor<'tcx>>(&self, visitor: &mut V) -> bool {
+ self.self_ty.visit_with(visitor) ||
+ self.trait_ref.map(|r| r.visit_with(visitor)).unwrap_or(false) ||
+ self.predicates.iter().any(|p| p.visit_with(visitor))
+ }
+}
+
 impl<'tcx> TypeFoldable<'tcx> for ty::Region {
  fn super_fold_with<F: TypeFolder<'tcx>>(&self, _folder: &mut F) -> Self {
  *self

src/librustc_typeck/coherence/mod.rs

@@ -35,7 +35,9 @@ use CrateCtxt;
 use middle::infer::{self, InferCtxt, TypeOrigin, new_infer_ctxt};
 use std::cell::RefCell;
 use std::rc::Rc;
+use syntax::ast;
 use syntax::codemap::Span;
+use syntax::errors::DiagnosticBuilder;
 use util::nodemap::{DefIdMap, FnvHashMap};
 use rustc::dep_graph::DepNode;
 use rustc::front::map as hir_map;
@@ -519,6 +521,13 @@ fn enforce_trait_manually_implementable(tcx: &TyCtxt, sp: Span, trait_def_id: De
  err.emit();
 }
 
+// Factored out into helper because the error cannot be defined in multiple locations.
+pub fn report_duplicate_item<'tcx>(tcx: &TyCtxt<'tcx>, sp: Span, name: ast::Name)
+ -> DiagnosticBuilder<'tcx>
+{
+ struct_span_err!(tcx.sess, sp, E0201, "duplicate definitions with name `{}`:", name)
+}
+
 pub fn check_coherence(crate_context: &CrateCtxt) {
  let _task = crate_context.tcx.dep_graph.in_task(DepNode::Coherence);
  let infcx = new_infer_ctxt(crate_context.tcx, &crate_context.tcx.tables, None);