From 5d79e8c4c97388dd1201135b8d6cfadccfd3c8e3 Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Sat, 13 Jul 2019 18:02:00 +0300 Subject: [PATCH 01/13] reserve `impl From for T` this is necessary for never-type stabilization --- src/libcore/convert.rs | 6 +++++ src/librustc/traits/select.rs | 21 ++++++++++++--- src/librustc/ty/mod.rs | 28 +++++++++++++------- src/librustc_typeck/check/wfcheck.rs | 29 ++++++++++++--------- src/libsyntax/feature_gate/builtin_attrs.rs | 4 +++ src/libsyntax_pos/symbol.rs | 1 + src/test/ui/never-impl-is-reserved.rs | 12 +++++++++ src/test/ui/never-impl-is-reserved.stderr | 12 +++++++++ 8 files changed, 87 insertions(+), 26 deletions(-) create mode 100644 src/test/ui/never-impl-is-reserved.rs create mode 100644 src/test/ui/never-impl-is-reserved.stderr diff --git a/src/libcore/convert.rs b/src/libcore/convert.rs index 06f2b7bab12eb..82c20efa9f0d6 100644 --- a/src/libcore/convert.rs +++ b/src/libcore/convert.rs @@ -554,6 +554,12 @@ impl From for T { fn from(t: T) -> T { t } } +#[stable(feature = "convert_infallible", since = "1.34.0")] +#[cfg(not(boostrap_stdarch_ignore_this))] +#[rustc_reservation_impl] +impl From for T { + fn from(t: !) -> T { t } +} // TryFrom implies TryInto #[stable(feature = "try_from", since = "1.34.0")] diff --git a/src/librustc/traits/select.rs b/src/librustc/traits/select.rs index a54bc05f16961..debd946d7164f 100644 --- a/src/librustc/traits/select.rs +++ b/src/librustc/traits/select.rs @@ -50,6 +50,8 @@ use std::iter; use std::rc::Rc; use crate::util::nodemap::{FxHashMap, FxHashSet}; +use syntax::symbol::sym; + pub struct SelectionContext<'cx, 'tcx> { infcx: &'cx InferCtxt<'cx, 'tcx>, @@ -1326,8 +1328,8 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { (result, dep_node) } - // Treat negative impls as unimplemented - fn filter_negative_impls( + // Treat negative impls as unimplemented, and reservation impls as Ok(None) + fn filter_negative_and_reservation_impls( &self, candidate: SelectionCandidate<'tcx>, ) -> SelectionResult<'tcx, SelectionCandidate<'tcx>> { @@ -1337,6 +1339,10 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { { return Err(Unimplemented); } + + if self.tcx().has_attr(def_id, sym::rustc_reservation_impl) { + return Ok(None); + } } Ok(Some(candidate)) } @@ -1453,7 +1459,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { // Instead, we select the right impl now but report `Bar does // not implement Clone`. if candidates.len() == 1 { - return self.filter_negative_impls(candidates.pop().unwrap()); + return self.filter_negative_and_reservation_impls(candidates.pop().unwrap()); } // Winnow, but record the exact outcome of evaluation, which @@ -1528,7 +1534,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { } // Just one candidate left. - self.filter_negative_impls(candidates.pop().unwrap().candidate) + self.filter_negative_and_reservation_impls(candidates.pop().unwrap().candidate) } fn is_knowable<'o>(&mut self, stack: &TraitObligationStack<'o, 'tcx>) -> Option { @@ -3728,6 +3734,13 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { return Err(()); } + if self.intercrate.is_none() && + self.tcx().has_attr(impl_def_id, sym::rustc_reservation_impl) + { + debug!("match_impl: reservation impls only apply in intercrate mode"); + return Err(()); + } + debug!("match_impl: success impl_substs={:?}", impl_substs); Ok(Normalized { value: impl_substs, diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs index 8bb9648e031ef..c5cbe0d0dabe7 100644 --- a/src/librustc/ty/mod.rs +++ b/src/librustc/ty/mod.rs @@ -2911,7 +2911,13 @@ impl<'tcx> TyCtxt<'tcx> { return Some(ImplOverlapKind::Permitted); } - let is_legit = if self.features().overlapping_marker_traits { + if self.impl_polarity(def_id1) != self.impl_polarity(def_id2) { + debug!("impls_are_allowed_to_overlap({:?}, {:?}) - different polarities, None", + def_id1, def_id2); + return None; + } + + let is_marker_overlap = if self.features().overlapping_marker_traits { let trait1_is_empty = self.impl_trait_ref(def_id1) .map_or(false, |trait_ref| { self.associated_item_def_ids(trait_ref.def_id).is_empty() @@ -2920,22 +2926,24 @@ impl<'tcx> TyCtxt<'tcx> { .map_or(false, |trait_ref| { self.associated_item_def_ids(trait_ref.def_id).is_empty() }); - self.impl_polarity(def_id1) == self.impl_polarity(def_id2) - && trait1_is_empty - && trait2_is_empty + trait1_is_empty && trait2_is_empty } else { let is_marker_impl = |def_id: DefId| -> bool { let trait_ref = self.impl_trait_ref(def_id); trait_ref.map_or(false, |tr| self.trait_def(tr.def_id).is_marker) }; - self.impl_polarity(def_id1) == self.impl_polarity(def_id2) - && is_marker_impl(def_id1) - && is_marker_impl(def_id2) + is_marker_impl(def_id1) && is_marker_impl(def_id2) }; - if is_legit { - debug!("impls_are_allowed_to_overlap({:?}, {:?}) = Some(Permitted)", - def_id1, def_id2); + // `#[rustc_reservation_impl]` impls don't overlap with anything + let is_reserve_overlap = { + self.has_attr(def_id1, sym::rustc_reservation_impl) || + self.has_attr(def_id2, sym::rustc_reservation_impl) + }; + + if is_marker_overlap || is_reserve_overlap { + debug!("impls_are_allowed_to_overlap({:?}, {:?}) = Some(Permitted) ({:?}/{:?})", + def_id1, def_id2, is_marker_overlap, is_reserve_overlap); Some(ImplOverlapKind::Permitted) } else { if let Some(self_ty1) = self.issue33140_self_ty(def_id1) { diff --git a/src/librustc_typeck/check/wfcheck.rs b/src/librustc_typeck/check/wfcheck.rs index b0e886a2aa2eb..87c34d62bde0a 100644 --- a/src/librustc_typeck/check/wfcheck.rs +++ b/src/librustc_typeck/check/wfcheck.rs @@ -398,18 +398,23 @@ fn check_impl<'tcx>( match *ast_trait_ref { Some(ref ast_trait_ref) => { - let trait_ref = fcx.tcx.impl_trait_ref(item_def_id).unwrap(); - let trait_ref = - fcx.normalize_associated_types_in( - ast_trait_ref.path.span, &trait_ref); - let obligations = - ty::wf::trait_obligations(fcx, - fcx.param_env, - fcx.body_id, - &trait_ref, - ast_trait_ref.path.span); - for obligation in obligations { - fcx.register_predicate(obligation); + // `#[rustc_reservation_impl]` impls are not real impls and + // therefore don't need to be WF (the trait's `Self: Trait` predicate + // won't hold). + if !fcx.tcx.has_attr(item_def_id, sym::rustc_reservation_impl) { + let trait_ref = fcx.tcx.impl_trait_ref(item_def_id).unwrap(); + let trait_ref = + fcx.normalize_associated_types_in( + ast_trait_ref.path.span, &trait_ref); + let obligations = + ty::wf::trait_obligations(fcx, + fcx.param_env, + fcx.body_id, + &trait_ref, + ast_trait_ref.path.span); + for obligation in obligations { + fcx.register_predicate(obligation); + } } } None => { diff --git a/src/libsyntax/feature_gate/builtin_attrs.rs b/src/libsyntax/feature_gate/builtin_attrs.rs index b6e13200f32af..d38489a2f0db9 100644 --- a/src/libsyntax/feature_gate/builtin_attrs.rs +++ b/src/libsyntax/feature_gate/builtin_attrs.rs @@ -498,6 +498,10 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[ overflow checking behavior of several libcore functions that are inlined \ across crates and will never be stable", ), + rustc_attr!(rustc_reservation_impl, Normal, template!(Word), + "the `#[rustc_reservation_impl]` attribute is internally used \ + for reserving for `for From for T` impl" + ), rustc_attr!( rustc_test_marker, Normal, template!(Word), "the `#[rustc_test_marker]` attribute is used internally to track tests", diff --git a/src/libsyntax_pos/symbol.rs b/src/libsyntax_pos/symbol.rs index 597ae83572cee..32af930ffb884 100644 --- a/src/libsyntax_pos/symbol.rs +++ b/src/libsyntax_pos/symbol.rs @@ -606,6 +606,7 @@ symbols! { rustc_std_internal_symbol, rustc_symbol_name, rustc_synthetic, + rustc_reservation_impl, rustc_test_marker, rustc_then_this_would_need, rustc_variance, diff --git a/src/test/ui/never-impl-is-reserved.rs b/src/test/ui/never-impl-is-reserved.rs new file mode 100644 index 0000000000000..9d16015bdc129 --- /dev/null +++ b/src/test/ui/never-impl-is-reserved.rs @@ -0,0 +1,12 @@ +// check that the `for T: From` impl is reserved + +#![feature(never_type)] + +pub struct MyFoo; +pub trait MyTrait {} + +impl MyTrait for MyFoo {} +// This will conflict with the first impl if we impl `for T: From`. +impl MyTrait for T where T: From {} //~ ERROR conflicting implementation + +fn main() {} diff --git a/src/test/ui/never-impl-is-reserved.stderr b/src/test/ui/never-impl-is-reserved.stderr new file mode 100644 index 0000000000000..09116eb4f7faf --- /dev/null +++ b/src/test/ui/never-impl-is-reserved.stderr @@ -0,0 +1,12 @@ +error[E0119]: conflicting implementations of trait `MyTrait` for type `MyFoo`: + --> $DIR/never-impl-is-reserved.rs:10:1 + | +LL | impl MyTrait for MyFoo {} + | ---------------------- first implementation here +LL | // This will conflict with the first impl if we impl `for T: From`. +LL | impl MyTrait for T where T: From {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `MyFoo` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0119`. From 9a94ecde04306986dac1b7ca88b4b327b67ea499 Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Sat, 13 Jul 2019 22:52:57 +0300 Subject: [PATCH 02/13] improve and add tests --- ...rved.rs => never-from-impl-is-reserved.rs} | 0 ...err => never-from-impl-is-reserved.stderr} | 2 +- .../reservation-impl-coherence-conflict.rs | 16 +++++++++++ ...reservation-impl-coherence-conflict.stderr | 11 ++++++++ .../reservation-impl-no-use.rs | 14 ++++++++++ .../reservation-impl-no-use.stderr | 15 ++++++++++ .../reservation-impls/reservation-impl-ok.rs | 28 +++++++++++++++++++ 7 files changed, 85 insertions(+), 1 deletion(-) rename src/test/ui/{never-impl-is-reserved.rs => never-from-impl-is-reserved.rs} (100%) rename src/test/ui/{never-impl-is-reserved.stderr => never-from-impl-is-reserved.stderr} (91%) create mode 100644 src/test/ui/traits/reservation-impls/reservation-impl-coherence-conflict.rs create mode 100644 src/test/ui/traits/reservation-impls/reservation-impl-coherence-conflict.stderr create mode 100644 src/test/ui/traits/reservation-impls/reservation-impl-no-use.rs create mode 100644 src/test/ui/traits/reservation-impls/reservation-impl-no-use.stderr create mode 100644 src/test/ui/traits/reservation-impls/reservation-impl-ok.rs diff --git a/src/test/ui/never-impl-is-reserved.rs b/src/test/ui/never-from-impl-is-reserved.rs similarity index 100% rename from src/test/ui/never-impl-is-reserved.rs rename to src/test/ui/never-from-impl-is-reserved.rs diff --git a/src/test/ui/never-impl-is-reserved.stderr b/src/test/ui/never-from-impl-is-reserved.stderr similarity index 91% rename from src/test/ui/never-impl-is-reserved.stderr rename to src/test/ui/never-from-impl-is-reserved.stderr index 09116eb4f7faf..7e9b21a542933 100644 --- a/src/test/ui/never-impl-is-reserved.stderr +++ b/src/test/ui/never-from-impl-is-reserved.stderr @@ -1,5 +1,5 @@ error[E0119]: conflicting implementations of trait `MyTrait` for type `MyFoo`: - --> $DIR/never-impl-is-reserved.rs:10:1 + --> $DIR/never-from-impl-is-reserved.rs:10:1 | LL | impl MyTrait for MyFoo {} | ---------------------- first implementation here diff --git a/src/test/ui/traits/reservation-impls/reservation-impl-coherence-conflict.rs b/src/test/ui/traits/reservation-impls/reservation-impl-coherence-conflict.rs new file mode 100644 index 0000000000000..1a5266f5583d6 --- /dev/null +++ b/src/test/ui/traits/reservation-impls/reservation-impl-coherence-conflict.rs @@ -0,0 +1,16 @@ +// compile-fail + +// check that reservation impls are accounted for in negative reasoning. + +#![feature(rustc_attrs)] + +trait MyTrait {} +#[rustc_reservation_impl] +impl MyTrait for () {} + +trait OtherTrait {} +impl OtherTrait for () {} +impl OtherTrait for T {} +//~^ ERROR conflicting implementations + +fn main() {} diff --git a/src/test/ui/traits/reservation-impls/reservation-impl-coherence-conflict.stderr b/src/test/ui/traits/reservation-impls/reservation-impl-coherence-conflict.stderr new file mode 100644 index 0000000000000..7b88d2b42db1b --- /dev/null +++ b/src/test/ui/traits/reservation-impls/reservation-impl-coherence-conflict.stderr @@ -0,0 +1,11 @@ +error[E0119]: conflicting implementations of trait `OtherTrait` for type `()`: + --> $DIR/reservation-impl-coherence-conflict.rs:13:1 + | +LL | impl OtherTrait for () {} + | ---------------------- first implementation here +LL | impl OtherTrait for T {} + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `()` + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0119`. diff --git a/src/test/ui/traits/reservation-impls/reservation-impl-no-use.rs b/src/test/ui/traits/reservation-impls/reservation-impl-no-use.rs new file mode 100644 index 0000000000000..f08338bdc1c91 --- /dev/null +++ b/src/test/ui/traits/reservation-impls/reservation-impl-no-use.rs @@ -0,0 +1,14 @@ +// compile-fail + +// check that reservation impls can't be used as normal impls in positive reasoning. + +#![feature(rustc_attrs)] + +trait MyTrait { fn foo(&self); } +#[rustc_reservation_impl] +impl MyTrait for () { fn foo(&self) {} } + +fn main() { + <() as MyTrait>::foo(&()); + //~^ ERROR the trait bound `(): MyTrait` is not satisfied +} diff --git a/src/test/ui/traits/reservation-impls/reservation-impl-no-use.stderr b/src/test/ui/traits/reservation-impls/reservation-impl-no-use.stderr new file mode 100644 index 0000000000000..8a86f53086dd5 --- /dev/null +++ b/src/test/ui/traits/reservation-impls/reservation-impl-no-use.stderr @@ -0,0 +1,15 @@ +error[E0277]: the trait bound `(): MyTrait` is not satisfied + --> $DIR/reservation-impl-no-use.rs:12:5 + | +LL | trait MyTrait { fn foo(&self); } + | -------------- required by `MyTrait::foo` +... +LL | <() as MyTrait>::foo(&()); + | ^^^^^^^^^^^^^^^^^^^^ the trait `MyTrait` is not implemented for `()` + | + = help: the following implementations were found: + <() as MyTrait> + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0277`. diff --git a/src/test/ui/traits/reservation-impls/reservation-impl-ok.rs b/src/test/ui/traits/reservation-impls/reservation-impl-ok.rs new file mode 100644 index 0000000000000..febd14b792297 --- /dev/null +++ b/src/test/ui/traits/reservation-impls/reservation-impl-ok.rs @@ -0,0 +1,28 @@ +// run-pass + +// rpass test for reservation impls. Not 100% required because `From` uses them, +// but still. + +#![feature(rustc_attrs)] + +use std::mem; + +trait MyTrait { + fn foo(&self, s: S) -> usize; +} + +#[rustc_reservation_impl] +impl MyTrait for T { + fn foo(&self, _x: u64) -> usize { 0 } +} + +// reservation impls don't create coherence conflicts, even with +// non-chain overlap. +impl MyTrait for u32 { + fn foo(&self, _x: S) -> usize { mem::size_of::() } +} + +fn main() { + // ...and the non-reservation impl gets picked.XS + assert_eq!(0u32.foo(0u64), mem::size_of::()); +} From 1ec7ae14fa5b4b29f56d7085f632dd6301ad4815 Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Sun, 14 Jul 2019 00:09:46 +0300 Subject: [PATCH 03/13] resolve the rustc_reservation_impl attribute in 1 place --- src/librustc/query/mod.rs | 2 +- src/librustc/traits/auto_trait.rs | 2 +- src/librustc/traits/select.rs | 24 ++++++------- src/librustc/ty/mod.rs | 46 ++++++++++++++++-------- src/librustc_metadata/decoder.rs | 2 +- src/librustc_metadata/encoder.rs | 3 +- src/librustc_metadata/schema.rs | 2 +- src/librustc_traits/lowering/mod.rs | 5 +-- src/librustc_typeck/check/wfcheck.rs | 53 +++++++++++++++------------- src/librustc_typeck/collect.rs | 26 ++++++++++++-- src/librustdoc/clean/mod.rs | 13 ++++--- 11 files changed, 112 insertions(+), 66 deletions(-) diff --git a/src/librustc/query/mod.rs b/src/librustc/query/mod.rs index c7260945295a6..b937d1a30409a 100644 --- a/src/librustc/query/mod.rs +++ b/src/librustc/query/mod.rs @@ -286,7 +286,7 @@ rustc_queries! { query associated_item(_: DefId) -> ty::AssocItem {} query impl_trait_ref(_: DefId) -> Option> {} - query impl_polarity(_: DefId) -> hir::ImplPolarity {} + query impl_polarity(_: DefId) -> ty::ImplPolarity {} query issue33140_self_ty(_: DefId) -> Option> {} } diff --git a/src/librustc/traits/auto_trait.rs b/src/librustc/traits/auto_trait.rs index d89cf8eb3e843..c481943e25e37 100644 --- a/src/librustc/traits/auto_trait.rs +++ b/src/librustc/traits/auto_trait.rs @@ -321,7 +321,7 @@ impl AutoTraitFinder<'tcx> { match vtable { Vtable::VtableImpl(VtableImplData { impl_def_id, .. }) => { // Blame tidy for the weird bracket placement - if infcx.tcx.impl_polarity(*impl_def_id) == hir::ImplPolarity::Negative + if infcx.tcx.impl_polarity(*impl_def_id) == ty::ImplPolarity::Negative { debug!("evaluate_nested_obligations: Found explicit negative impl\ {:?}, bailing out", impl_def_id); diff --git a/src/librustc/traits/select.rs b/src/librustc/traits/select.rs index debd946d7164f..61bb53dd334fa 100644 --- a/src/librustc/traits/select.rs +++ b/src/librustc/traits/select.rs @@ -50,8 +50,6 @@ use std::iter; use std::rc::Rc; use crate::util::nodemap::{FxHashMap, FxHashSet}; -use syntax::symbol::sym; - pub struct SelectionContext<'cx, 'tcx> { infcx: &'cx InferCtxt<'cx, 'tcx>, @@ -1334,15 +1332,15 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { candidate: SelectionCandidate<'tcx>, ) -> SelectionResult<'tcx, SelectionCandidate<'tcx>> { if let ImplCandidate(def_id) = candidate { - if !self.allow_negative_impls - && self.tcx().impl_polarity(def_id) == hir::ImplPolarity::Negative - { - return Err(Unimplemented); - } - - if self.tcx().has_attr(def_id, sym::rustc_reservation_impl) { - return Ok(None); - } + match self.tcx().impl_polarity(def_id) { + ty::ImplPolarity::Negative if !self.allow_negative_impls => { + return Err(Unimplemented); + } + ty::ImplPolarity::Reservation => { + return Ok(None); + } + _ => {} + }; } Ok(Some(candidate)) } @@ -3734,8 +3732,8 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { return Err(()); } - if self.intercrate.is_none() && - self.tcx().has_attr(impl_def_id, sym::rustc_reservation_impl) + if self.intercrate.is_none() + && self.tcx().impl_polarity(impl_def_id) == ty::ImplPolarity::Reservation { debug!("match_impl: reservation impls only apply in intercrate mode"); return Err(()); diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs index c5cbe0d0dabe7..b546a24534666 100644 --- a/src/librustc/ty/mod.rs +++ b/src/librustc/ty/mod.rs @@ -167,6 +167,16 @@ pub struct ImplHeader<'tcx> { pub predicates: Vec>, } +#[derive(Copy, Clone, PartialEq, RustcEncodable, RustcDecodable, HashStable)] +pub enum ImplPolarity { + /// `impl Trait for Type` + Positive, + /// `impl !Trait for Type` + Negative, + /// `#[rustc_reservation_impl] impl Trait for Type` + Reservation, +} + #[derive(Copy, Clone, Debug, PartialEq, HashStable)] pub struct AssocItem { pub def_id: DefId, @@ -2911,11 +2921,24 @@ impl<'tcx> TyCtxt<'tcx> { return Some(ImplOverlapKind::Permitted); } - if self.impl_polarity(def_id1) != self.impl_polarity(def_id2) { - debug!("impls_are_allowed_to_overlap({:?}, {:?}) - different polarities, None", - def_id1, def_id2); - return None; - } + match (self.impl_polarity(def_id1), self.impl_polarity(def_id2)) { + (ImplPolarity::Reservation, _) | + (_, ImplPolarity::Reservation) => { + // `#[rustc_reservation_impl]` impls don't overlap with anything + debug!("impls_are_allowed_to_overlap({:?}, {:?}) = Some(Permitted) (reservations)", + def_id1, def_id2); + return Some(ImplOverlapKind::Permitted); + } + (ImplPolarity::Positive, ImplPolarity::Negative) | + (ImplPolarity::Negative, ImplPolarity::Positive) => { + // FIXME: when can this happen? + debug!("impls_are_allowed_to_overlap({:?}, {:?}) - None (differing polarities)", + def_id1, def_id2); + return None; + } + (ImplPolarity::Positive, ImplPolarity::Positive) | + (ImplPolarity::Negative, ImplPolarity::Negative) => {} + }; let is_marker_overlap = if self.features().overlapping_marker_traits { let trait1_is_empty = self.impl_trait_ref(def_id1) @@ -2935,15 +2958,10 @@ impl<'tcx> TyCtxt<'tcx> { is_marker_impl(def_id1) && is_marker_impl(def_id2) }; - // `#[rustc_reservation_impl]` impls don't overlap with anything - let is_reserve_overlap = { - self.has_attr(def_id1, sym::rustc_reservation_impl) || - self.has_attr(def_id2, sym::rustc_reservation_impl) - }; - if is_marker_overlap || is_reserve_overlap { - debug!("impls_are_allowed_to_overlap({:?}, {:?}) = Some(Permitted) ({:?}/{:?})", - def_id1, def_id2, is_marker_overlap, is_reserve_overlap); + if is_marker_overlap { + debug!("impls_are_allowed_to_overlap({:?}, {:?}) = Some(Permitted) (marker overlap)", + def_id1, def_id2); Some(ImplOverlapKind::Permitted) } else { if let Some(self_ty1) = self.issue33140_self_ty(def_id1) { @@ -3325,7 +3343,7 @@ fn issue33140_self_ty(tcx: TyCtxt<'_>, def_id: DefId) -> Option> { debug!("issue33140_self_ty({:?}), trait-ref={:?}", def_id, trait_ref); let is_marker_like = - tcx.impl_polarity(def_id) == hir::ImplPolarity::Positive && + tcx.impl_polarity(def_id) == ty::ImplPolarity::Positive && tcx.associated_item_def_ids(trait_ref.def_id).is_empty(); // Check whether these impls would be ok for a marker trait. diff --git a/src/librustc_metadata/decoder.rs b/src/librustc_metadata/decoder.rs index 34c84b1d79d4b..9785b69eaf09e 100644 --- a/src/librustc_metadata/decoder.rs +++ b/src/librustc_metadata/decoder.rs @@ -722,7 +722,7 @@ impl<'a, 'tcx> CrateMetadata { self.get_impl_data(id).parent_impl } - pub fn get_impl_polarity(&self, id: DefIndex) -> hir::ImplPolarity { + pub fn get_impl_polarity(&self, id: DefIndex) -> ty::ImplPolarity { self.get_impl_data(id).polarity } diff --git a/src/librustc_metadata/encoder.rs b/src/librustc_metadata/encoder.rs index f430f01542efe..9bf0eefd9fe70 100644 --- a/src/librustc_metadata/encoder.rs +++ b/src/librustc_metadata/encoder.rs @@ -1172,8 +1172,9 @@ impl EncodeContext<'tcx> { ctor_sig: None, }), repr_options) } - hir::ItemKind::Impl(_, polarity, defaultness, ..) => { + hir::ItemKind::Impl(_, _, defaultness, ..) => { let trait_ref = tcx.impl_trait_ref(def_id); + let polarity = tcx.impl_polarity(def_id); let parent = if let Some(trait_ref) = trait_ref { let trait_def = tcx.trait_def(trait_ref.def_id); trait_def.ancestors(tcx, def_id).nth(1).and_then(|node| { diff --git a/src/librustc_metadata/schema.rs b/src/librustc_metadata/schema.rs index 1a5f0e17ba7ce..90967c7933323 100644 --- a/src/librustc_metadata/schema.rs +++ b/src/librustc_metadata/schema.rs @@ -327,7 +327,7 @@ pub struct TraitAliasData<'tcx> { #[derive(RustcEncodable, RustcDecodable)] pub struct ImplData<'tcx> { - pub polarity: hir::ImplPolarity, + pub polarity: ty::ImplPolarity, pub defaultness: hir::Defaultness, pub parent_impl: Option, diff --git a/src/librustc_traits/lowering/mod.rs b/src/librustc_traits/lowering/mod.rs index 1558ce1bced52..51d49f0d59ae7 100644 --- a/src/librustc_traits/lowering/mod.rs +++ b/src/librustc_traits/lowering/mod.rs @@ -4,7 +4,7 @@ use rustc::hir::def::DefKind; use rustc::hir::def_id::DefId; use rustc::hir::intravisit::{self, NestedVisitorMap, Visitor}; use rustc::hir::map::definitions::DefPathData; -use rustc::hir::{self, ImplPolarity}; +use rustc::hir; use rustc::traits::{ Clause, Clauses, @@ -295,7 +295,8 @@ fn program_clauses_for_trait(tcx: TyCtxt<'_>, def_id: DefId) -> Clauses<'_> { } fn program_clauses_for_impl(tcx: TyCtxt<'tcx>, def_id: DefId) -> Clauses<'tcx> { - if let ImplPolarity::Negative = tcx.impl_polarity(def_id) { + // FIXME: implement reservation impls. + if let ty::ImplPolarity::Negative = tcx.impl_polarity(def_id) { return List::empty(); } diff --git a/src/librustc_typeck/check/wfcheck.rs b/src/librustc_typeck/check/wfcheck.rs index 87c34d62bde0a..e0e878ffdcc47 100644 --- a/src/librustc_typeck/check/wfcheck.rs +++ b/src/librustc_typeck/check/wfcheck.rs @@ -94,20 +94,27 @@ pub fn check_item_well_formed(tcx: TyCtxt<'_>, def_id: DefId) { // // won't be allowed unless there's an *explicit* implementation of `Send` // for `T` - hir::ItemKind::Impl(_, polarity, defaultness, _, ref trait_ref, ref self_ty, _) => { + hir::ItemKind::Impl(_, _, defaultness, _, ref trait_ref, ref self_ty, _) => { let is_auto = tcx.impl_trait_ref(tcx.hir().local_def_id(item.hir_id)) - .map_or(false, |trait_ref| tcx.trait_is_auto(trait_ref.def_id)); + .map_or(false, |trait_ref| tcx.trait_is_auto(trait_ref.def_id)); + let polarity = tcx.impl_polarity(def_id); if let (hir::Defaultness::Default { .. }, true) = (defaultness, is_auto) { tcx.sess.span_err(item.span, "impls of auto traits cannot be default"); } - if polarity == hir::ImplPolarity::Positive { - check_impl(tcx, item, self_ty, trait_ref); - } else { - // FIXME(#27579): what amount of WF checking do we need for neg impls? - if trait_ref.is_some() && !is_auto { - span_err!(tcx.sess, item.span, E0192, - "negative impls are only allowed for \ - auto traits (e.g., `Send` and `Sync`)") + match polarity { + ty::ImplPolarity::Positive => { + check_impl(tcx, item, self_ty, trait_ref); + } + ty::ImplPolarity::Negative => { + // FIXME(#27579): what amount of WF checking do we need for neg impls? + if trait_ref.is_some() && !is_auto { + span_err!(tcx.sess, item.span, E0192, + "negative impls are only allowed for \ + auto traits (e.g., `Send` and `Sync`)") + } + } + ty::ImplPolarity::Reservation => { + // FIXME: what amount of WF checking do we need for reservation impls? } } } @@ -401,20 +408,18 @@ fn check_impl<'tcx>( // `#[rustc_reservation_impl]` impls are not real impls and // therefore don't need to be WF (the trait's `Self: Trait` predicate // won't hold). - if !fcx.tcx.has_attr(item_def_id, sym::rustc_reservation_impl) { - let trait_ref = fcx.tcx.impl_trait_ref(item_def_id).unwrap(); - let trait_ref = - fcx.normalize_associated_types_in( - ast_trait_ref.path.span, &trait_ref); - let obligations = - ty::wf::trait_obligations(fcx, - fcx.param_env, - fcx.body_id, - &trait_ref, - ast_trait_ref.path.span); - for obligation in obligations { - fcx.register_predicate(obligation); - } + let trait_ref = fcx.tcx.impl_trait_ref(item_def_id).unwrap(); + let trait_ref = + fcx.normalize_associated_types_in( + ast_trait_ref.path.span, &trait_ref); + let obligations = + ty::wf::trait_obligations(fcx, + fcx.param_env, + fcx.body_id, + &trait_ref, + ast_trait_ref.path.span); + for obligation in obligations { + fcx.register_predicate(obligation); } } None => { diff --git a/src/librustc_typeck/collect.rs b/src/librustc_typeck/collect.rs index d2e9203779cc8..4503bb264a5f7 100644 --- a/src/librustc_typeck/collect.rs +++ b/src/librustc_typeck/collect.rs @@ -1866,10 +1866,30 @@ fn impl_trait_ref(tcx: TyCtxt<'_>, def_id: DefId) -> Option> { } } -fn impl_polarity(tcx: TyCtxt<'_>, def_id: DefId) -> hir::ImplPolarity { +fn impl_polarity(tcx: TyCtxt<'_>, def_id: DefId) -> ty::ImplPolarity { let hir_id = tcx.hir().as_local_hir_id(def_id).unwrap(); - match tcx.hir().expect_item(hir_id).node { - hir::ItemKind::Impl(_, polarity, ..) => polarity, + let is_rustc_reservation = tcx.has_attr(def_id, sym::rustc_reservation_impl); + let item = tcx.hir().expect_item(hir_id); + match &item.node { + hir::ItemKind::Impl(_, hir::ImplPolarity::Negative, ..) => { + if is_rustc_reservation { + tcx.sess.span_err(item.span, "reservation impls can't be negative"); + } + ty::ImplPolarity::Negative + } + hir::ItemKind::Impl(_, hir::ImplPolarity::Positive, _, _, None, _, _) => { + if is_rustc_reservation { + tcx.sess.span_err(item.span, "reservation impls can't be inherent"); + } + ty::ImplPolarity::Positive + } + hir::ItemKind::Impl(_, hir::ImplPolarity::Positive, _, _, Some(_tr), _, _) => { + if is_rustc_reservation { + ty::ImplPolarity::Reservation + } else { + ty::ImplPolarity::Positive + } + } ref item => bug!("impl_polarity: {:?} not an impl", item), } } diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index ae70fdc530be6..15ada0952c8a3 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -3864,11 +3864,13 @@ pub enum ImplPolarity { Negative, } -impl Clean for hir::ImplPolarity { +impl Clean for ty::ImplPolarity { fn clean(&self, _: &DocContext<'_>) -> ImplPolarity { match self { - &hir::ImplPolarity::Positive => ImplPolarity::Positive, - &hir::ImplPolarity::Negative => ImplPolarity::Negative, + &ty::ImplPolarity::Positive | + // FIXME: do we want to do something else here? + &ty::ImplPolarity::Reservation => ImplPolarity::Positive, + &ty::ImplPolarity::Negative => ImplPolarity::Negative, } } } @@ -3900,6 +3902,7 @@ impl Clean> for doctree::Impl<'_> { let mut ret = Vec::new(); let trait_ = self.trait_.clean(cx); let items = self.items.iter().map(|ii| ii.clean(cx)).collect::>(); + let def_id = cx.tcx.hir().local_def_id(self.id); // If this impl block is an implementation of the Deref trait, then we // need to try inlining the target's inherent impl blocks as well. @@ -3918,7 +3921,7 @@ impl Clean> for doctree::Impl<'_> { name: None, attrs: self.attrs.clean(cx), source: self.whence.clean(cx), - def_id: cx.tcx.hir().local_def_id(self.id), + def_id, visibility: self.vis.clean(cx), stability: cx.stability(self.id).clean(cx), deprecation: cx.deprecation(self.id).clean(cx), @@ -3929,7 +3932,7 @@ impl Clean> for doctree::Impl<'_> { trait_, for_: self.for_.clean(cx), items, - polarity: Some(self.polarity.clean(cx)), + polarity: Some(cx.tcx.impl_polarity(def_id).clean(cx)), synthetic: false, blanket_impl: None, }) From b5665e811ba4eca0f778efb65bd3e4a69f4c3ca6 Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Sat, 27 Jul 2019 20:44:14 +0300 Subject: [PATCH 04/13] improve comments --- src/librustc/traits/select.rs | 2 +- src/librustc/ty/mod.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustc/traits/select.rs b/src/librustc/traits/select.rs index 61bb53dd334fa..de1c71a1ba31f 100644 --- a/src/librustc/traits/select.rs +++ b/src/librustc/traits/select.rs @@ -1326,7 +1326,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { (result, dep_node) } - // Treat negative impls as unimplemented, and reservation impls as Ok(None) + // Treat negative impls as unimplemented, and reservation impls as ambiguity. fn filter_negative_and_reservation_impls( &self, candidate: SelectionCandidate<'tcx>, diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs index b546a24534666..6a9d7eb0750ba 100644 --- a/src/librustc/ty/mod.rs +++ b/src/librustc/ty/mod.rs @@ -2931,7 +2931,7 @@ impl<'tcx> TyCtxt<'tcx> { } (ImplPolarity::Positive, ImplPolarity::Negative) | (ImplPolarity::Negative, ImplPolarity::Positive) => { - // FIXME: when can this happen? + // `impl AutoTrait for Type` + `impl !AutoTrait for Type` debug!("impls_are_allowed_to_overlap({:?}, {:?}) - None (differing polarities)", def_id1, def_id2); return None; From 9196b2d0c89b8af853f8f41b5854655932491758 Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Sat, 27 Jul 2019 22:18:34 +0300 Subject: [PATCH 05/13] add error message for case --- src/libcore/convert.rs | 7 ++++- src/librustc/traits/select.rs | 31 +++++++++++++++++-- src/libsyntax/feature_gate/builtin_attrs.rs | 3 +- .../ui/never-from-impl-is-reserved.stderr | 2 ++ .../reservation-impl-coherence-conflict.rs | 2 +- ...reservation-impl-coherence-conflict.stderr | 2 ++ .../reservation-impl-no-use.rs | 2 +- .../reservation-impls/reservation-impl-ok.rs | 2 +- 8 files changed, 43 insertions(+), 8 deletions(-) diff --git a/src/libcore/convert.rs b/src/libcore/convert.rs index 82c20efa9f0d6..6edd56f749290 100644 --- a/src/libcore/convert.rs +++ b/src/libcore/convert.rs @@ -556,7 +556,12 @@ impl From for T { #[stable(feature = "convert_infallible", since = "1.34.0")] #[cfg(not(boostrap_stdarch_ignore_this))] -#[rustc_reservation_impl] +#[rustc_reservation_impl="a future version of Rust might implement `From` for \ + all types. \ + However, it is OK to implement `From` for types you own - \ + when the blanket impl will be added, coherence will be changed \ + to make these impls not be an error." +] impl From for T { fn from(t: !) -> T { t } } diff --git a/src/librustc/traits/select.rs b/src/librustc/traits/select.rs index de1c71a1ba31f..c91ee1b9caa93 100644 --- a/src/librustc/traits/select.rs +++ b/src/librustc/traits/select.rs @@ -43,6 +43,8 @@ use crate::hir; use rustc_data_structures::bit_set::GrowableBitSet; use rustc_data_structures::sync::Lock; use rustc_target::spec::abi::Abi; +use syntax::attr; +use syntax::symbol::sym; use std::cell::{Cell, RefCell}; use std::cmp; use std::fmt::{self, Display}; @@ -99,6 +101,9 @@ pub enum IntercrateAmbiguityCause { trait_desc: String, self_desc: Option, }, + ReservationImpl { + message: String + }, } impl IntercrateAmbiguityCause { @@ -139,6 +144,11 @@ impl IntercrateAmbiguityCause { trait_desc, self_desc ) } + &IntercrateAmbiguityCause::ReservationImpl { + ref message + } => { + message.clone() + } } } } @@ -1328,15 +1338,32 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { // Treat negative impls as unimplemented, and reservation impls as ambiguity. fn filter_negative_and_reservation_impls( - &self, + &mut self, candidate: SelectionCandidate<'tcx>, ) -> SelectionResult<'tcx, SelectionCandidate<'tcx>> { if let ImplCandidate(def_id) = candidate { - match self.tcx().impl_polarity(def_id) { + let tcx = self.tcx(); + match tcx.impl_polarity(def_id) { ty::ImplPolarity::Negative if !self.allow_negative_impls => { return Err(Unimplemented); } ty::ImplPolarity::Reservation => { + if let Some(intercrate_ambiguity_clauses) + = &mut self.intercrate_ambiguity_causes + { + let attrs = tcx.get_attrs(def_id); + let attr = attr::find_by_name(&attrs, sym::rustc_reservation_impl); + let value = attr.and_then(|a| a.value_str()); + if let Some(value) = value { + debug!("filter_negative_and_reservation_impls: \ + reservation impl ambiguity on {:?}", def_id); + intercrate_ambiguity_clauses.push( + IntercrateAmbiguityCause::ReservationImpl { + message: value.to_string() + } + ); + } + } return Ok(None); } _ => {} diff --git a/src/libsyntax/feature_gate/builtin_attrs.rs b/src/libsyntax/feature_gate/builtin_attrs.rs index d38489a2f0db9..d14afc6deaa69 100644 --- a/src/libsyntax/feature_gate/builtin_attrs.rs +++ b/src/libsyntax/feature_gate/builtin_attrs.rs @@ -457,7 +457,6 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[ // ========================================================================== // Internal attributes, Misc: // ========================================================================== - gated!( lang, Normal, template!(NameValueStr: "name"), lang_items, "language items are subject to change", @@ -498,7 +497,7 @@ pub const BUILTIN_ATTRIBUTES: &[BuiltinAttribute] = &[ overflow checking behavior of several libcore functions that are inlined \ across crates and will never be stable", ), - rustc_attr!(rustc_reservation_impl, Normal, template!(Word), + rustc_attr!(rustc_reservation_impl, Normal, template!(NameValueStr: "reservation message"), "the `#[rustc_reservation_impl]` attribute is internally used \ for reserving for `for From for T` impl" ), diff --git a/src/test/ui/never-from-impl-is-reserved.stderr b/src/test/ui/never-from-impl-is-reserved.stderr index 7e9b21a542933..352fed7ca4515 100644 --- a/src/test/ui/never-from-impl-is-reserved.stderr +++ b/src/test/ui/never-from-impl-is-reserved.stderr @@ -6,6 +6,8 @@ LL | impl MyTrait for MyFoo {} LL | // This will conflict with the first impl if we impl `for T: From`. LL | impl MyTrait for T where T: From {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `MyFoo` + | + = note: a future version of Rust might implement `From` for all types. However, it is OK to implement `From` for types you own - when the blanket impl will be added, coherence will be changed to make these impls not be an error. error: aborting due to previous error diff --git a/src/test/ui/traits/reservation-impls/reservation-impl-coherence-conflict.rs b/src/test/ui/traits/reservation-impls/reservation-impl-coherence-conflict.rs index 1a5266f5583d6..775278c30cd4c 100644 --- a/src/test/ui/traits/reservation-impls/reservation-impl-coherence-conflict.rs +++ b/src/test/ui/traits/reservation-impls/reservation-impl-coherence-conflict.rs @@ -5,7 +5,7 @@ #![feature(rustc_attrs)] trait MyTrait {} -#[rustc_reservation_impl] +#[rustc_reservation_impl="this impl is reserved"] impl MyTrait for () {} trait OtherTrait {} diff --git a/src/test/ui/traits/reservation-impls/reservation-impl-coherence-conflict.stderr b/src/test/ui/traits/reservation-impls/reservation-impl-coherence-conflict.stderr index 7b88d2b42db1b..47e141bd048eb 100644 --- a/src/test/ui/traits/reservation-impls/reservation-impl-coherence-conflict.stderr +++ b/src/test/ui/traits/reservation-impls/reservation-impl-coherence-conflict.stderr @@ -5,6 +5,8 @@ LL | impl OtherTrait for () {} | ---------------------- first implementation here LL | impl OtherTrait for T {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `()` + | + = note: this impl is reserved error: aborting due to previous error diff --git a/src/test/ui/traits/reservation-impls/reservation-impl-no-use.rs b/src/test/ui/traits/reservation-impls/reservation-impl-no-use.rs index f08338bdc1c91..3391daaabe975 100644 --- a/src/test/ui/traits/reservation-impls/reservation-impl-no-use.rs +++ b/src/test/ui/traits/reservation-impls/reservation-impl-no-use.rs @@ -5,7 +5,7 @@ #![feature(rustc_attrs)] trait MyTrait { fn foo(&self); } -#[rustc_reservation_impl] +#[rustc_reservation_impl = "foo"] impl MyTrait for () { fn foo(&self) {} } fn main() { diff --git a/src/test/ui/traits/reservation-impls/reservation-impl-ok.rs b/src/test/ui/traits/reservation-impls/reservation-impl-ok.rs index febd14b792297..611c8d8841323 100644 --- a/src/test/ui/traits/reservation-impls/reservation-impl-ok.rs +++ b/src/test/ui/traits/reservation-impls/reservation-impl-ok.rs @@ -11,7 +11,7 @@ trait MyTrait { fn foo(&self, s: S) -> usize; } -#[rustc_reservation_impl] +#[rustc_reservation_impl = "foo"] impl MyTrait for T { fn foo(&self, _x: u64) -> usize { 0 } } From d7eb5620804540ce5f48d49fadcf0930d9d24000 Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Sat, 14 Sep 2019 15:34:42 +0300 Subject: [PATCH 06/13] add test for lattice specialization --- .../reservation-impl-non-lattice-ok.rs | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 src/test/ui/traits/reservation-impls/reservation-impl-non-lattice-ok.rs diff --git a/src/test/ui/traits/reservation-impls/reservation-impl-non-lattice-ok.rs b/src/test/ui/traits/reservation-impls/reservation-impl-non-lattice-ok.rs new file mode 100644 index 0000000000000..2517052b5e4d7 --- /dev/null +++ b/src/test/ui/traits/reservation-impls/reservation-impl-non-lattice-ok.rs @@ -0,0 +1,56 @@ +// build-pass + +// Check that a reservation impl does not force other impls to follow +// a lattice discipline. + +// Why did we ever want to do this? +// +// We want to eventually add a `impl From for T` impl. That impl conflicts +// with existing impls - at least the `impl From for T` impl. There are +// 2 ways we thought of for dealing with that conflict: +// +// 1. Using specialization and doing some handling for the overlap. The current +// thought is for something like "lattice specialization", which means providing +// an (higher-priority) impl for the intersection of every 2 conflicting impls +// that determines what happens in the intersection case. That's the first +// thing we thought about - see e.g. +// https://github.com/rust-lang/rust/issues/57012#issuecomment-452150775 +// +// 2. The other way is to notice that `impl From for T` is basically a marker +// trait, as you say since its only method is uninhabited, and allow for "marker +// trait overlap", where the conflict "doesn't matter" as there is nothing that +// can cause a conflict. +// +// Now it turned out lattice specialization doesn't work it, because an +// `impl From for Smaht` would require a `impl From for Smaht`, +// breaking backwards-compatibility in a fairly painful way. So if we want to +// go with a known approach, we should go with a "marker trait overlap"-style +// approach. + +#![feature(rustc_attrs, never_type)] + +trait MyTrait {} + +impl MyTrait for ! {} + +trait MyFrom { + fn my_from(x: T) -> Self; +} + +// Given the "normal" impls for From +#[rustc_reservation_impl="this impl is reserved"] +impl MyFrom for T { + fn my_from(x: !) -> Self { match x {} } +} + +impl MyFrom for T { + fn my_from(x: T) -> Self { x } +} + +// ... we *do* want to allow this common pattern, of `From for MySmaht` +struct MySmaht(T); +impl MyFrom for MySmaht { + fn my_from(x: T) -> Self { MySmaht(x) } +} + +fn main() {} From 5de1fafb1569ad12f039562ca86f14730d970a3b Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Wed, 18 Sep 2019 19:37:26 +0300 Subject: [PATCH 07/13] improve comment --- .../reservation-impls/reservation-impl-non-lattice-ok.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/test/ui/traits/reservation-impls/reservation-impl-non-lattice-ok.rs b/src/test/ui/traits/reservation-impls/reservation-impl-non-lattice-ok.rs index 2517052b5e4d7..ae1556cb4dc88 100644 --- a/src/test/ui/traits/reservation-impls/reservation-impl-non-lattice-ok.rs +++ b/src/test/ui/traits/reservation-impls/reservation-impl-non-lattice-ok.rs @@ -16,10 +16,10 @@ // thing we thought about - see e.g. // https://github.com/rust-lang/rust/issues/57012#issuecomment-452150775 // -// 2. The other way is to notice that `impl From for T` is basically a marker -// trait, as you say since its only method is uninhabited, and allow for "marker -// trait overlap", where the conflict "doesn't matter" as there is nothing that -// can cause a conflict. +// 2. The other way is to notice that `impl From for T` is basically a +// marker trait since its only method is uninhabited, and allow for "marker +// trait overlap", where the conflict "doesn't matter" because it can't +// actually cause any ambiguity. // // Now it turned out lattice specialization doesn't work it, because an // `impl From for Smaht` would require a `impl From for Smaht`, From 68fd593a22c98246857726f576a14651d9ecb997 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Mon, 23 Sep 2019 13:51:59 -0400 Subject: [PATCH 08/13] cite reservation impls tracking issue --- src/librustc/ty/mod.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs index 6a9d7eb0750ba..5a436b28fbcf3 100644 --- a/src/librustc/ty/mod.rs +++ b/src/librustc/ty/mod.rs @@ -174,6 +174,9 @@ pub enum ImplPolarity { /// `impl !Trait for Type` Negative, /// `#[rustc_reservation_impl] impl Trait for Type` + /// + /// This is a "stability hack", not a real Rust feature. + /// See #64631 for details. Reservation, } From b40a64da4fb1f2012bd925100b4cff2a38a72b30 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Mon, 23 Sep 2019 13:52:16 -0400 Subject: [PATCH 09/13] remove outdated fixme --- src/librustc_traits/lowering/mod.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/librustc_traits/lowering/mod.rs b/src/librustc_traits/lowering/mod.rs index 51d49f0d59ae7..4c30227150fb1 100644 --- a/src/librustc_traits/lowering/mod.rs +++ b/src/librustc_traits/lowering/mod.rs @@ -295,7 +295,6 @@ fn program_clauses_for_trait(tcx: TyCtxt<'_>, def_id: DefId) -> Clauses<'_> { } fn program_clauses_for_impl(tcx: TyCtxt<'tcx>, def_id: DefId) -> Clauses<'tcx> { - // FIXME: implement reservation impls. if let ty::ImplPolarity::Negative = tcx.impl_polarity(def_id) { return List::empty(); } From da60c53fa7fb74d314d3ad4f33ea55479884313a Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Mon, 23 Sep 2019 13:52:24 -0400 Subject: [PATCH 10/13] nit: update text to avoid "lattice specialization" term --- .../reservation-impl-non-lattice-ok.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/test/ui/traits/reservation-impls/reservation-impl-non-lattice-ok.rs b/src/test/ui/traits/reservation-impls/reservation-impl-non-lattice-ok.rs index ae1556cb4dc88..f14589ccf846d 100644 --- a/src/test/ui/traits/reservation-impls/reservation-impl-non-lattice-ok.rs +++ b/src/test/ui/traits/reservation-impls/reservation-impl-non-lattice-ok.rs @@ -9,11 +9,12 @@ // with existing impls - at least the `impl From for T` impl. There are // 2 ways we thought of for dealing with that conflict: // -// 1. Using specialization and doing some handling for the overlap. The current -// thought is for something like "lattice specialization", which means providing -// an (higher-priority) impl for the intersection of every 2 conflicting impls -// that determines what happens in the intersection case. That's the first -// thing we thought about - see e.g. +// 1. Using specialization and doing some handling for the +// overlap. The current thought is to require ["intersection +// impls"][ii], specialization", which means providing an +// (higher-priority) impl for the intersection of every 2 conflicting +// impls that determines what happens in the intersection case. That's +// the first thing we thought about - see e.g. // https://github.com/rust-lang/rust/issues/57012#issuecomment-452150775 // // 2. The other way is to notice that `impl From for T` is basically a @@ -26,6 +27,8 @@ // breaking backwards-compatibility in a fairly painful way. So if we want to // go with a known approach, we should go with a "marker trait overlap"-style // approach. +// +// [ii]: http://smallcultfollowing.com/babysteps/blog/2016/09/24/intersection-impls/ #![feature(rustc_attrs, never_type)] From 167ab0439eb9f2d7b199f846fe07061b31185e09 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Mon, 23 Sep 2019 14:27:34 -0400 Subject: [PATCH 11/13] nit: update error text to cite tracking issue --- src/libcore/convert.rs | 8 ++------ src/test/ui/never-from-impl-is-reserved.stderr | 2 +- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/src/libcore/convert.rs b/src/libcore/convert.rs index 6edd56f749290..71edea561b8bb 100644 --- a/src/libcore/convert.rs +++ b/src/libcore/convert.rs @@ -556,12 +556,8 @@ impl From for T { #[stable(feature = "convert_infallible", since = "1.34.0")] #[cfg(not(boostrap_stdarch_ignore_this))] -#[rustc_reservation_impl="a future version of Rust might implement `From` for \ - all types. \ - However, it is OK to implement `From` for types you own - \ - when the blanket impl will be added, coherence will be changed \ - to make these impls not be an error." -] +#[rustc_reservation_impl="permitting this impl would forbid us from adding \ +`impl From for T` later; see rust-lang/rust#64715 for details"] impl From for T { fn from(t: !) -> T { t } } diff --git a/src/test/ui/never-from-impl-is-reserved.stderr b/src/test/ui/never-from-impl-is-reserved.stderr index 352fed7ca4515..8b8d0f4ea73be 100644 --- a/src/test/ui/never-from-impl-is-reserved.stderr +++ b/src/test/ui/never-from-impl-is-reserved.stderr @@ -7,7 +7,7 @@ LL | // This will conflict with the first impl if we impl `for T: From`. LL | impl MyTrait for T where T: From {} | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `MyFoo` | - = note: a future version of Rust might implement `From` for all types. However, it is OK to implement `From` for types you own - when the blanket impl will be added, coherence will be changed to make these impls not be an error. + = note: permitting this impl would forbid us from adding `impl From for T` later; see rust-lang/rust#64715 for details error: aborting due to previous error From 99dc545552c40212b9d062512cd391af076b51c9 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Tue, 24 Sep 2019 11:10:42 -0400 Subject: [PATCH 12/13] add a rustdoc comment to the reservation impl --- src/libcore/convert.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/libcore/convert.rs b/src/libcore/convert.rs index 71edea561b8bb..f639309db8791 100644 --- a/src/libcore/convert.rs +++ b/src/libcore/convert.rs @@ -554,6 +554,11 @@ impl From for T { fn from(t: T) -> T { t } } +/// **Stability note:** This impl does not yet exist, but we are +/// "reserving space" to add it in the future. See +/// [rust-lang/rust#64715][#64715] for details. +/// +/// [#64715]: https://github.com/rust-lang/rust/issues/64715 #[stable(feature = "convert_infallible", since = "1.34.0")] #[cfg(not(boostrap_stdarch_ignore_this))] #[rustc_reservation_impl="permitting this impl would forbid us from adding \ From e70724c23bd2bd5cfbbac784d103f2a61a40284f Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Tue, 24 Sep 2019 21:16:09 +0300 Subject: [PATCH 13/13] address rebase damage --- src/libcore/convert.rs | 2 +- .../traits/reservation-impls/reservation-impl-no-use.stderr | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libcore/convert.rs b/src/libcore/convert.rs index f639309db8791..3cd2337ee59a5 100644 --- a/src/libcore/convert.rs +++ b/src/libcore/convert.rs @@ -560,7 +560,7 @@ impl From for T { /// /// [#64715]: https://github.com/rust-lang/rust/issues/64715 #[stable(feature = "convert_infallible", since = "1.34.0")] -#[cfg(not(boostrap_stdarch_ignore_this))] +#[cfg(not(bootstrap))] #[rustc_reservation_impl="permitting this impl would forbid us from adding \ `impl From for T` later; see rust-lang/rust#64715 for details"] impl From for T { diff --git a/src/test/ui/traits/reservation-impls/reservation-impl-no-use.stderr b/src/test/ui/traits/reservation-impls/reservation-impl-no-use.stderr index 8a86f53086dd5..0cd56b978f10c 100644 --- a/src/test/ui/traits/reservation-impls/reservation-impl-no-use.stderr +++ b/src/test/ui/traits/reservation-impls/reservation-impl-no-use.stderr @@ -1,11 +1,11 @@ error[E0277]: the trait bound `(): MyTrait` is not satisfied - --> $DIR/reservation-impl-no-use.rs:12:5 + --> $DIR/reservation-impl-no-use.rs:12:26 | LL | trait MyTrait { fn foo(&self); } | -------------- required by `MyTrait::foo` ... LL | <() as MyTrait>::foo(&()); - | ^^^^^^^^^^^^^^^^^^^^ the trait `MyTrait` is not implemented for `()` + | ^^^ the trait `MyTrait` is not implemented for `()` | = help: the following implementations were found: <() as MyTrait>