Skip to content

Commit

Permalink
Forbid items with the same name being defined in overlapping inherent
Browse files Browse the repository at this point in the history
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.

Closes rust-lang#22889
  • Loading branch information
aturon committed Mar 11, 2016
1 parent 9cc3bfc commit 21df87f
Show file tree
Hide file tree
Showing 9 changed files with 113 additions and 45 deletions.
1 change: 1 addition & 0 deletions src/librustc/dep_graph/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ pub enum DepNode {
CoherenceCheckImpl(DefId),
CoherenceOverlapCheck(DefId),
CoherenceOverlapCheckSpecial(DefId),
CoherenceOverlapInherentCheck(DefId),
CoherenceOrphanCheck(DefId),
Variance,
WfCheck(DefId),
Expand Down
8 changes: 4 additions & 4 deletions src/librustc/middle/infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -482,14 +482,14 @@ pub fn mk_sub_poly_trait_refs<'a, 'tcx>(cx: &InferCtxt<'a, 'tcx>,
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>)
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, a_ref, b_ref),
(None, None) => mk_eqty(cx, a_is_expected, a.self_ty, b.self_ty),
(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"),
}
}
Expand Down
21 changes: 9 additions & 12 deletions src/librustc/middle/traits/coherence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,15 @@

//! See `README.md` for high-level documentation

use super::{Normalized, SelectionContext};
use super::{Obligation, ObligationCause, 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::ty::error::TypeError;
use middle::infer::{self, InferCtxt, TypeOrigin};
use syntax::codemap::{DUMMY_SP, Span};
use syntax::codemap::DUMMY_SP;

#[derive(Copy, Clone)]
struct InferIsLocal(bool);
Expand All @@ -31,7 +28,7 @@ struct InferIsLocal(bool);
pub fn overlapping_impls<'cx, 'tcx>(infcx: &InferCtxt<'cx, 'tcx>,
impl1_def_id: DefId,
impl2_def_id: DefId)
-> Option<ImplTy<'tcx>>
-> Option<ty::ImplHeader<'tcx>>
{
debug!("impl_can_satisfy(\
impl1_def_id={:?}, \
Expand All @@ -48,7 +45,7 @@ pub fn overlapping_impls<'cx, 'tcx>(infcx: &InferCtxt<'cx, 'tcx>,
fn overlap<'cx, 'tcx>(selcx: &mut SelectionContext<'cx, 'tcx>,
a_def_id: DefId,
b_def_id: DefId)
-> Option<ImplHeader<'tcx>>
-> Option<ty::ImplHeader<'tcx>>
{
debug!("overlap(a_def_id={:?}, b_def_id={:?})",
a_def_id,
Expand All @@ -64,8 +61,8 @@ fn overlap<'cx, 'tcx>(selcx: &mut SelectionContext<'cx, 'tcx>,
if let Err(_) = infer::mk_eq_impl_headers(selcx.infcx(),
true,
TypeOrigin::Misc(DUMMY_SP),
a_impl_header,
b_impl_header) {
&a_impl_header,
&b_impl_header) {
return None;
}

Expand All @@ -74,7 +71,7 @@ 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_impl_header.prediates
a_impl_header.predicates
.iter()
.chain(&b_impl_header.predicates)
.map(|p| infcx.resolve_type_vars_if_possible(p))
Expand Down
16 changes: 8 additions & 8 deletions src/librustc/middle/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,23 +164,23 @@ pub struct ImplHeader<'tcx> {
}

impl<'tcx> ImplHeader<'tcx> {
pub fn with_fresh_ty_vars<'a,'tcx>(selcx: &mut traits::SelectionContext<'a,'tcx>,
impl_def_id: DefId)
-> 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),
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),
}.subst(tcx, impl_substs);
predicates: tcx.lookup_predicates(impl_def_id).predicates.into_vec(),
}.subst(tcx, &impl_substs);

let Normalized { value: mut header, obligations: obligations } =
proect::normalize(selcx, ObligationCause::dummy(), &header);
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
Expand Down
3 changes: 1 addition & 2 deletions src/librustc/middle/ty/structural_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -452,8 +452,7 @@ impl<'tcx> TypeFoldable<'tcx> for ty::ImplHeader<'tcx> {
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.into_iter().map(|p| p.fold_with(folder)).collect(),
polarity: self.polarity,
predicates: self.predicates.iter().map(|p| p.fold_with(folder)).collect(),
}
}

Expand Down
9 changes: 9 additions & 0 deletions src/librustc_typeck/coherence/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
72 changes: 64 additions & 8 deletions src/librustc_typeck/coherence/overlap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
// except according to those terms.

//! Overlap: No two impls for the same trait are implemented for the
//! same type.
//! same type. Likewise, no two inherent impls for a given type
//! constructor provide a method with the same name.

use middle::cstore::{CrateStore, LOCAL_CRATE};
use middle::def_id::DefId;
Expand Down Expand Up @@ -115,7 +116,6 @@ impl<'cx, 'tcx> OverlapChecker<'cx, 'tcx> {
}
}


fn check_if_impls_overlap(&self,
impl1_def_id: DefId,
impl2_def_id: DefId)
Expand All @@ -128,8 +128,8 @@ impl<'cx, 'tcx> OverlapChecker<'cx, 'tcx> {
impl2_def_id);

let infcx = infer::new_infer_ctxt(self.tcx, &self.tcx.tables, None);
if let Some(trait_ref) = traits::overlapping_impls(&infcx, impl1_def_id, impl2_def_id) {
self.report_overlap_error(impl1_def_id, impl2_def_id, trait_ref);
if let Some(header) = traits::overlapping_impls(&infcx, impl1_def_id, impl2_def_id) {
self.report_overlap_error(impl1_def_id, impl2_def_id, header.trait_ref.unwrap());
}
}
}
Expand All @@ -150,13 +150,13 @@ impl<'cx, 'tcx> OverlapChecker<'cx, 'tcx> {
}).unwrap_or(String::new())
};

let mut err = struct_span_err!(self.tcx.sess, self.span_of_impl(impl1), E0119,
let mut err = struct_span_err!(self.tcx.sess, self.span_of_def_id(impl1), E0119,
"conflicting implementations of trait `{}`{}:",
trait_ref,
self_type);

if impl2.is_local() {
span_note!(&mut err, self.span_of_impl(impl2),
span_note!(&mut err, self.span_of_def_id(impl2),
"conflicting implementation is here:");
} else {
let cname = self.tcx.sess.cstore.crate_name(impl2.krate);
Expand All @@ -165,10 +165,61 @@ impl<'cx, 'tcx> OverlapChecker<'cx, 'tcx> {
err.emit();
}

fn span_of_impl(&self, impl_did: DefId) -> Span {
let node_id = self.tcx.map.as_local_node_id(impl_did).unwrap();
fn span_of_def_id(&self, did: DefId) -> Span {
let node_id = self.tcx.map.as_local_node_id(did).unwrap();
self.tcx.map.span(node_id)
}

fn check_for_common_items_in_impls(&self, impl1: DefId, impl2: DefId) {
#[derive(Copy, Clone, PartialEq)]
enum Namespace { Type, Value }

fn name_and_namespace(tcx: &TyCtxt, item: &ty::ImplOrTraitItemId)
-> (ast::Name, Namespace)
{
let name = tcx.impl_or_trait_item(item.def_id()).name();
(name, match *item {
ty::TypeTraitItemId(..) => Namespace::Type,
ty::ConstTraitItemId(..) => Namespace::Value,
ty::MethodTraitItemId(..) => Namespace::Value,
})
}

let impl_items = self.tcx.impl_items.borrow();

for item1 in &impl_items[&impl1] {
let (name, namespace) = name_and_namespace(&self.tcx, item1);

for item2 in &impl_items[&impl2] {
if (name, namespace) == name_and_namespace(&self.tcx, item2) {
let mut err = super::report_duplicate_item(
&self.tcx, self.span_of_def_id(item1.def_id()), name);
span_note!(&mut err, self.span_of_def_id(item2.def_id()),
"conflicting definition is here:");
err.emit();
}
}
}
}

fn check_for_overlapping_inherent_impls(&self, ty_def_id: DefId) {
let _task = self.tcx.dep_graph.in_task(DepNode::CoherenceOverlapInherentCheck(ty_def_id));

let inherent_impls = self.tcx.inherent_impls.borrow();
let impls = match inherent_impls.get(&ty_def_id) {
Some(impls) => impls,
None => return
};

for (i, &impl1_def_id) in impls.iter().enumerate() {
for &impl2_def_id in &impls[(i+1)..] {
let infcx = infer::new_infer_ctxt(self.tcx, &self.tcx.tables, None);
if traits::overlapping_impls(&infcx, impl1_def_id, impl2_def_id).is_some() {
self.check_for_common_items_in_impls(impl1_def_id, impl2_def_id)
}
}
}
}
}


Expand All @@ -180,6 +231,11 @@ impl<'cx, 'tcx,'v> intravisit::Visitor<'v> for OverlapChecker<'cx, 'tcx> {
self.check_for_overlapping_impls_of_trait(trait_def_id);
}

hir::ItemEnum(..) | hir::ItemStruct(..) => {
let type_def_id = self.tcx.map.local_def_id(item.id);
self.check_for_overlapping_inherent_impls(type_def_id);
}

hir::ItemDefaultImpl(..) => {
// look for another default impl; note that due to the
// general orphan/coherence rules, it must always be
Expand Down
13 changes: 2 additions & 11 deletions src/librustc_typeck/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ use lint;
use middle::def::Def;
use middle::def_id::DefId;
use constrained_type_params as ctp;
use coherence;
use middle::lang_items::SizedTraitLangItem;
use middle::resolve_lifetime;
use middle::const_eval::{self, ConstVal};
Expand Down Expand Up @@ -750,17 +751,7 @@ fn convert_item(ccx: &CrateCtxt, it: &hir::Item) {
_ => &mut seen_value_items,
};
if !seen_items.insert(impl_item.name) {
let desc = match impl_item.node {
hir::ImplItemKind::Const(_, _) => "associated constant",
hir::ImplItemKind::Type(_) => "associated type",
hir::ImplItemKind::Method(ref sig, _) =>
match sig.explicit_self.node {
hir::SelfStatic => "associated function",
_ => "method",
},
};

span_err!(tcx.sess, impl_item.span, E0201, "duplicate {}", desc);
coherence::report_duplicate_item(tcx, impl_item.span, impl_item.name).emit();
}

if let hir::ImplItemKind::Const(ref ty, _) = impl_item.node {
Expand Down
15 changes: 15 additions & 0 deletions src/librustc_typeck/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2285,6 +2285,21 @@ impl Baz for Foo {
type Quux = u32;
}
```
Note, however, that items with the same name are allowed for inherent `impl`
blocks that don't overlap:
```
struct Foo<T>(T);
impl Foo<u8> {
fn bar(&self) -> bool { self.0 > 5 }
}
impl Foo<bool> {
fn bar(&self) -> bool { self.0 }
}
```
"##,

E0202: r##"
Expand Down

0 comments on commit 21df87f

Please sign in to comment.