From 462c83e272e2ba268aaf11ef00e9d47c52011b90 Mon Sep 17 00:00:00 2001 From: Aaron Turon Date: Sat, 30 Jan 2016 09:25:25 -0800 Subject: [PATCH] Address basic nits from initial review --- src/librustc/middle/traits/specialize.rs | 17 ++++--- src/librustc/middle/ty/trait_def.rs | 20 +++++--- src/librustc_typeck/coherence/overlap.rs | 4 +- src/libsyntax/feature_gate.rs | 1 + .../specialization-negative-impl.rs | 2 + .../compile-fail/specialization-no-default.rs | 49 +++++++++++++++++++ .../specialization-default-methods.rs | 23 +++++++++ 7 files changed, 97 insertions(+), 19 deletions(-) diff --git a/src/librustc/middle/traits/specialize.rs b/src/librustc/middle/traits/specialize.rs index 91ddcc3b9e4..2c501c1a481 100644 --- a/src/librustc/middle/traits/specialize.rs +++ b/src/librustc/middle/traits/specialize.rs @@ -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>, } @@ -70,13 +69,13 @@ pub fn new() -> 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![]; @@ -98,6 +97,7 @@ pub fn insert<'tcx>(&mut self, return Err(Overlap { with_impl: possible_sibling, on_trait_ref: trait_ref, + in_context: infcx, }); } @@ -118,6 +118,7 @@ pub fn insert<'tcx>(&mut self, return Err(Overlap { with_impl: possible_sibling, on_trait_ref: trait_ref, + in_context: infcx, }); } diff --git a/src/librustc/middle/ty/trait_def.rs b/src/librustc/middle/ty/trait_def.rs index 737bb873d6d..85cea4d8096 100644 --- a/src/librustc/middle/ty/trait_def.rs +++ b/src/librustc/middle/ty/trait_def.rs @@ -125,7 +125,8 @@ fn read_trait_impls(&self, tcx: &TyCtxt<'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); @@ -161,7 +162,9 @@ pub fn record_local_impl(&self, 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. @@ -174,6 +177,8 @@ pub fn record_remote_impl(&self, 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 @@ -186,15 +191,14 @@ pub fn record_remote_impl(&self, /// 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. diff --git a/src/librustc_typeck/coherence/overlap.rs b/src/librustc_typeck/coherence/overlap.rs index 8f125abaec0..1ebc131224d 100644 --- a/src/librustc_typeck/coherence/overlap.rs +++ b/src/librustc_typeck/coherence/overlap.rs @@ -133,9 +133,7 @@ fn visit_item(&mut self, item: &'v hir::Item) { 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 { diff --git a/src/libsyntax/feature_gate.rs b/src/libsyntax/feature_gate.rs index d50eb17c87b..a5f0fbbd094 100644 --- a/src/libsyntax/feature_gate.rs +++ b/src/libsyntax/feature_gate.rs @@ -250,6 +250,7 @@ ("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) diff --git a/src/test/compile-fail/specialization-negative-impl.rs b/src/test/compile-fail/specialization-negative-impl.rs index 3a907dc3b8e..9aec614012e 100644 --- a/src/test/compile-fail/specialization-negative-impl.rs +++ b/src/test/compile-fail/specialization-negative-impl.rs @@ -13,6 +13,8 @@ struct TestType(T); +// TODO: nail down the rules here with @nikomatsakis + unsafe impl Send for TestType {} impl !Send for TestType {} diff --git a/src/test/compile-fail/specialization-no-default.rs b/src/test/compile-fail/specialization-no-default.rs index 143c6f0e858..06a01f0ca05 100644 --- a/src/test/compile-fail/specialization-no-default.rs +++ b/src/test/compile-fail/specialization-no-default.rs @@ -10,6 +10,10 @@ #![feature(specialization)] +//////////////////////////////////////////////////////////////////////////////// +// Test 1: one layer of specialization, multiple methods, missing `default` +//////////////////////////////////////////////////////////////////////////////// + trait Foo { fn foo(&self); fn bar(&self); @@ -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; } @@ -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 Baz for T { + default fn baz(&self) {} +} + +impl 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 Redundant for T { + default fn redundant(&self) {} +} + +impl Redundant for T { + fn redundant(&self) {} +} + +impl Redundant for i32 { + default fn redundant(&self) {} +} + fn main() {} diff --git a/src/test/run-pass/specialization-default-methods.rs b/src/test/run-pass/specialization-default-methods.rs index 9c52ceb5247..d662c5bfa28 100644 --- a/src/test/run-pass/specialization-default-methods.rs +++ b/src/test/run-pass/specialization-default-methods.rs @@ -16,6 +16,12 @@ trait Foo { fn foo(&self) -> bool; } +// Specialization tree for Foo: +// +// T +// / \ +// i32 i64 + impl Foo for T { default fn foo(&self) -> bool { false } } @@ -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 +// /\ +// / \ +// Vec $Vec + impl Bar for T {} // use the provided method impl Bar for i32 { -- GitLab