Skip to content

Commit

Permalink
Address basic nits from initial review
Browse files Browse the repository at this point in the history
  • Loading branch information
aturon committed Mar 14, 2016
1 parent 9734406 commit 462c83e
Show file tree
Hide file tree
Showing 7 changed files with 97 additions and 19 deletions.
17 changes: 9 additions & 8 deletions src/librustc/middle/traits/specialize.rs
Expand Up @@ -52,10 +52,9 @@ pub struct SpecializationGraph {
}

/// Information pertinent to an overlapping impl error.
pub struct Overlap<'tcx> {
pub struct Overlap<'a, 'tcx: 'a> {
pub in_context: InferCtxt<'a, 'tcx>,
pub with_impl: DefId,

/// NB: this TraitRef can contain inference variables!
pub on_trait_ref: ty::TraitRef<'tcx>,
}

Expand All @@ -70,13 +69,13 @@ impl SpecializationGraph {
/// Insert a local impl into the specialization graph. If an existing impl
/// conflicts with it (has overlap, but neither specializes the other),
/// information about the area of overlap is returned in the `Err`.
pub fn insert<'tcx>(&mut self,
tcx: &ty::ctxt<'tcx>,
impl_def_id: DefId,
trait_ref: ty::TraitRef)
-> Result<(), Overlap<'tcx>> {
pub fn insert<'a, 'tcx>(&mut self,
tcx: &'a ty::ctxt<'tcx>,
impl_def_id: DefId)
-> Result<(), Overlap<'a, 'tcx>> {
assert!(impl_def_id.is_local());

let trait_ref = tcx.impl_trait_ref(impl_def_id).unwrap();
let mut parent = trait_ref.def_id;
let mut my_children = vec![];

Expand All @@ -98,6 +97,7 @@ impl SpecializationGraph {
return Err(Overlap {
with_impl: possible_sibling,
on_trait_ref: trait_ref,
in_context: infcx,
});
}

Expand All @@ -118,6 +118,7 @@ impl SpecializationGraph {
return Err(Overlap {
with_impl: possible_sibling,
on_trait_ref: trait_ref,
in_context: infcx,
});
}

Expand Down
20 changes: 12 additions & 8 deletions src/librustc/middle/ty/trait_def.rs
Expand Up @@ -125,7 +125,8 @@ impl<'tcx> TraitDef<'tcx> {
fn record_impl(&self,
tcx: &TyCtxt<'tcx>,
impl_def_id: DefId,
impl_trait_ref: TraitRef<'tcx>) -> bool {
impl_trait_ref: TraitRef<'tcx>)
-> bool {
debug!("TraitDef::record_impl for {:?}, from {:?}",
self, impl_trait_ref);

Expand Down Expand Up @@ -161,7 +162,9 @@ impl<'tcx> TraitDef<'tcx> {
tcx: &TyCtxt<'tcx>,
impl_def_id: DefId,
impl_trait_ref: TraitRef<'tcx>) {
self.record_impl(tcx, impl_def_id, impl_trait_ref);
assert!(impl_def_id.is_local());
let was_new = self.record_impl(tcx, impl_def_id, impl_trait_ref);
assert!(was_new);
}

/// Records a trait-to-implementation mapping for a non-local impl.
Expand All @@ -174,6 +177,8 @@ impl<'tcx> TraitDef<'tcx> {
impl_def_id: DefId,
impl_trait_ref: TraitRef<'tcx>,
parent_impl: DefId) {
assert!(!impl_def_id.is_local());

// if the impl has not previously been recorded
if self.record_impl(tcx, impl_def_id, impl_trait_ref) {
// if the impl is non-local, it's placed directly into the
Expand All @@ -186,15 +191,14 @@ impl<'tcx> TraitDef<'tcx> {
/// Adds a local impl into the specialization graph, returning an error with
/// overlap information if the impl overlaps but does not specialize an
/// existing impl.
pub fn add_impl_for_specialization(&self,
tcx: &ctxt<'tcx>,
impl_def_id: DefId,
impl_trait_ref: TraitRef<'tcx>)
-> Result<(), traits::Overlap<'tcx>> {
pub fn add_impl_for_specialization<'a>(&self,
tcx: &'a ctxt<'tcx>,
impl_def_id: DefId)
-> Result<(), traits::Overlap<'a, 'tcx>> {
assert!(impl_def_id.is_local());

self.specialization_graph.borrow_mut()
.insert(tcx, impl_def_id, impl_trait_ref)
.insert(tcx, impl_def_id)
}

/// Returns the immediately less specialized impl, if any.
Expand Down
4 changes: 1 addition & 3 deletions src/librustc_typeck/coherence/overlap.rs
Expand Up @@ -133,9 +133,7 @@ impl<'cx, 'tcx,'v> intravisit::Visitor<'v> for OverlapChecker<'cx, 'tcx> {
let def = self.tcx.lookup_trait_def(trait_def_id);

// attempt to insert into the specialization graph
let insert_result = def.add_impl_for_specialization(self.tcx,
impl_def_id,
trait_ref);
let insert_result = def.add_impl_for_specialization(self.tcx, impl_def_id);

// insertion failed due to overlap
if let Err(overlap) = insert_result {
Expand Down
1 change: 1 addition & 0 deletions src/libsyntax/feature_gate.rs
Expand Up @@ -250,6 +250,7 @@ const KNOWN_FEATURES: &'static [(&'static str, &'static str, Option<u32>, Status
("question_mark", "1.9.0", Some(31436), Active)

// impl specialization (RFC 1210)
// TODO: update with issue number (once it exists), before landing
("specialization", "1.7.0", None, Active),
];
// (changing above list without updating src/doc/reference.md makes @cmr sad)
Expand Down
2 changes: 2 additions & 0 deletions src/test/compile-fail/specialization-negative-impl.rs
Expand Up @@ -13,6 +13,8 @@

struct TestType<T>(T);

// TODO: nail down the rules here with @nikomatsakis

unsafe impl<T> Send for TestType<T> {}
impl !Send for TestType<u8> {}

Expand Down
49 changes: 49 additions & 0 deletions src/test/compile-fail/specialization-no-default.rs
Expand Up @@ -10,6 +10,10 @@

#![feature(specialization)]

////////////////////////////////////////////////////////////////////////////////
// Test 1: one layer of specialization, multiple methods, missing `default`
////////////////////////////////////////////////////////////////////////////////

trait Foo {
fn foo(&self);
fn bar(&self);
Expand All @@ -28,6 +32,10 @@ impl Foo for u32 {
fn bar(&self) {} //~ ERROR E0520
}

////////////////////////////////////////////////////////////////////////////////
// Test 2: one layer of specialization, missing `default` on associated type
////////////////////////////////////////////////////////////////////////////////

trait Bar {
type T;
}
Expand All @@ -40,4 +48,45 @@ impl Bar for u8 {
type T = (); //~ ERROR E0520
}

////////////////////////////////////////////////////////////////////////////////
// Test 3a: multiple layers of specialization, missing interior `default`
////////////////////////////////////////////////////////////////////////////////

trait Baz {
fn baz(&self);
}

impl<T> Baz for T {
default fn baz(&self) {}
}

impl<T: Clone> Baz for T {
fn baz(&self) {}
}

impl Baz for i32 {
fn baz(&self) {}
}

////////////////////////////////////////////////////////////////////////////////
// Test 3b: multiple layers of specialization, missing interior `default`,
// redundant `default` in bottom layer.
////////////////////////////////////////////////////////////////////////////////

trait Redundant {
fn redundant(&self);
}

impl<T> Redundant for T {
default fn redundant(&self) {}
}

impl<T: Clone> Redundant for T {
fn redundant(&self) {}
}

impl Redundant for i32 {
default fn redundant(&self) {}
}

fn main() {}
23 changes: 23 additions & 0 deletions src/test/run-pass/specialization-default-methods.rs
Expand Up @@ -16,6 +16,12 @@ trait Foo {
fn foo(&self) -> bool;
}

// Specialization tree for Foo:
//
// T
// / \
// i32 i64

impl<T> Foo for T {
default fn foo(&self) -> bool { false }
}
Expand All @@ -38,6 +44,23 @@ trait Bar {
fn bar(&self) -> i32 { 0 }
}

// Specialization tree for Bar.
// Uses of $ designate that method is provided
//
// $Bar (the trait)
// |
// T
// /|\
// / | \
// / | \
// / | \
// / | \
// / | \
// $i32 &str $Vec<T>
// /\
// / \
// Vec<i32> $Vec<i64>

impl<T> Bar for T {} // use the provided method

impl Bar for i32 {
Expand Down

0 comments on commit 462c83e

Please sign in to comment.