Auto merge of #62661 - arielb1:never-reserve, r=nikomatsakis

reserve `impl<T> From<!> for T`

this is necessary for never-type stabilization.

cc #57012 #35121

I think we wanted a crater run for this @nikomatsakis?

r? @nikomatsakis
This commit is contained in:
bors 2019-09-26 08:42:34 +00:00
commit 134004f74d
22 changed files with 339 additions and 51 deletions

View File

@ -554,6 +554,18 @@ impl<T> From<T> 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(bootstrap))]
#[rustc_reservation_impl="permitting this impl would forbid us from adding \
`impl<T> From<!> for T` later; see rust-lang/rust#64715 for details"]
impl<T> From<!> for T {
fn from(t: !) -> T { t }
}
// TryFrom implies TryInto
#[stable(feature = "try_from", since = "1.34.0")]

View File

@ -290,7 +290,7 @@
query associated_item(_: DefId) -> ty::AssocItem {}
query impl_trait_ref(_: DefId) -> Option<ty::TraitRef<'tcx>> {}
query impl_polarity(_: DefId) -> hir::ImplPolarity {}
query impl_polarity(_: DefId) -> ty::ImplPolarity {}
query issue33140_self_ty(_: DefId) -> Option<ty::Ty<'tcx>> {}
}

View File

@ -321,7 +321,7 @@ fn evaluate_predicates(
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);

View File

@ -43,6 +43,8 @@
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<String>,
},
ReservationImpl {
message: String
},
}
impl IntercrateAmbiguityCause {
@ -139,6 +144,11 @@ pub fn intercrate_ambiguity_hint(&self) -> String {
trait_desc, self_desc
)
}
&IntercrateAmbiguityCause::ReservationImpl {
ref message
} => {
message.clone()
}
}
}
}
@ -1326,17 +1336,38 @@ fn in_task<OP, R>(&mut self, op: OP) -> (R, DepNodeIndex)
(result, dep_node)
}
// Treat negative impls as unimplemented
fn filter_negative_impls(
&self,
// Treat negative impls as unimplemented, and reservation impls as ambiguity.
fn filter_negative_and_reservation_impls(
&mut self,
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);
}
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);
}
_ => {}
};
}
Ok(Some(candidate))
}
@ -1453,7 +1484,7 @@ fn candidate_from_obligation_no_cache<'o>(
// 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 +1559,7 @@ fn candidate_from_obligation_no_cache<'o>(
}
// 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<Conflict> {
@ -3728,6 +3759,13 @@ fn match_impl(
return Err(());
}
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(());
}
debug!("match_impl: success impl_substs={:?}", impl_substs);
Ok(Normalized {
value: impl_substs,

View File

@ -167,6 +167,19 @@ pub struct ImplHeader<'tcx> {
pub predicates: Vec<Predicate<'tcx>>,
}
#[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`
///
/// This is a "stability hack", not a real Rust feature.
/// See #64631 for details.
Reservation,
}
#[derive(Copy, Clone, Debug, PartialEq, HashStable)]
pub struct AssocItem {
pub def_id: DefId,
@ -2911,7 +2924,26 @@ pub fn impls_are_allowed_to_overlap(self, def_id1: DefId, def_id2: DefId)
return Some(ImplOverlapKind::Permitted);
}
let is_legit = if self.features().overlapping_marker_traits {
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) => {
// `impl AutoTrait for Type` + `impl !AutoTrait for Type`
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)
.map_or(false, |trait_ref| {
self.associated_item_def_ids(trait_ref.def_id).is_empty()
@ -2920,22 +2952,19 @@ pub fn impls_are_allowed_to_overlap(self, def_id1: DefId, def_id2: DefId)
.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);
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) {
@ -3317,7 +3346,7 @@ fn issue33140_self_ty(tcx: TyCtxt<'_>, def_id: DefId) -> Option<Ty<'_>> {
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.

View File

@ -722,7 +722,7 @@ pub fn get_parent_impl(&self, id: DefIndex) -> Option<DefId> {
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
}

View File

@ -1175,8 +1175,9 @@ fn encode_info_for_item(&mut self, (def_id, item): (DefId, &'tcx hir::Item)) ->
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| {

View File

@ -328,7 +328,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<DefId>,

View File

@ -4,7 +4,7 @@
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,7 @@ 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) {
if let ty::ImplPolarity::Negative = tcx.impl_polarity(def_id) {
return List::empty();
}

View File

@ -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?
}
}
}
@ -398,16 +405,19 @@ fn check_impl<'tcx>(
match *ast_trait_ref {
Some(ref ast_trait_ref) => {
// `#[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).
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);
fcx.param_env,
fcx.body_id,
&trait_ref,
ast_trait_ref.path.span);
for obligation in obligations {
fcx.register_predicate(obligation);
}

View File

@ -1889,10 +1889,30 @@ fn impl_trait_ref(tcx: TyCtxt<'_>, def_id: DefId) -> Option<ty::TraitRef<'_>> {
}
}
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),
}
}

View File

@ -3855,11 +3855,13 @@ pub enum ImplPolarity {
Negative,
}
impl Clean<ImplPolarity> for hir::ImplPolarity {
impl Clean<ImplPolarity> 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,
}
}
}
@ -3891,6 +3893,7 @@ fn clean(&self, cx: &DocContext<'_>) -> Vec<Item> {
let mut ret = Vec::new();
let trait_ = self.trait_.clean(cx);
let items = self.items.iter().map(|ii| ii.clean(cx)).collect::<Vec<_>>();
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.
@ -3909,7 +3912,7 @@ fn clean(&self, cx: &DocContext<'_>) -> Vec<Item> {
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),
@ -3920,7 +3923,7 @@ fn clean(&self, cx: &DocContext<'_>) -> Vec<Item> {
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,
})

View File

@ -457,7 +457,6 @@ macro_rules! experimental {
// ==========================================================================
// Internal attributes, Misc:
// ==========================================================================
gated!(
lang, Normal, template!(NameValueStr: "name"), lang_items,
"language items are subject to change",
@ -498,6 +497,10 @@ macro_rules! experimental {
overflow checking behavior of several libcore functions that are inlined \
across crates and will never be stable",
),
rustc_attr!(rustc_reservation_impl, Normal, template!(NameValueStr: "reservation message"),
"the `#[rustc_reservation_impl]` attribute is internally used \
for reserving for `for<T> From<!> for T` impl"
),
rustc_attr!(
rustc_test_marker, Normal, template!(Word),
"the `#[rustc_test_marker]` attribute is used internally to track tests",

View File

@ -606,6 +606,7 @@
rustc_std_internal_symbol,
rustc_symbol_name,
rustc_synthetic,
rustc_reservation_impl,
rustc_test_marker,
rustc_then_this_would_need,
rustc_variance,

View File

@ -0,0 +1,12 @@
// check that the `for<T> 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> T: From<!>`.
impl<T> MyTrait for T where T: From<!> {} //~ ERROR conflicting implementation
fn main() {}

View File

@ -0,0 +1,14 @@
error[E0119]: conflicting implementations of trait `MyTrait` for type `MyFoo`:
--> $DIR/never-from-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> T: From<!>`.
LL | impl<T> MyTrait for T where T: From<!> {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `MyFoo`
|
= note: permitting this impl would forbid us from adding `impl<T> From<!> for T` later; see rust-lang/rust#64715 for details
error: aborting due to previous error
For more information about this error, try `rustc --explain E0119`.

View File

@ -0,0 +1,16 @@
// compile-fail
// check that reservation impls are accounted for in negative reasoning.
#![feature(rustc_attrs)]
trait MyTrait {}
#[rustc_reservation_impl="this impl is reserved"]
impl MyTrait for () {}
trait OtherTrait {}
impl OtherTrait for () {}
impl<T: MyTrait> OtherTrait for T {}
//~^ ERROR conflicting implementations
fn main() {}

View File

@ -0,0 +1,13 @@
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<T: MyTrait> OtherTrait for T {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `()`
|
= note: this impl is reserved
error: aborting due to previous error
For more information about this error, try `rustc --explain E0119`.

View File

@ -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 = "foo"]
impl MyTrait for () { fn foo(&self) {} }
fn main() {
<() as MyTrait>::foo(&());
//~^ ERROR the trait bound `(): MyTrait` is not satisfied
}

View File

@ -0,0 +1,15 @@
error[E0277]: the trait bound `(): MyTrait` is not satisfied
--> $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 `()`
|
= help: the following implementations were found:
<() as MyTrait>
error: aborting due to previous error
For more information about this error, try `rustc --explain E0277`.

View File

@ -0,0 +1,59 @@
// 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<T> From<!> for T` impl. That impl conflicts
// with existing impls - at least the `impl<T> From<T> 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 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
// 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<T> From<T> for Smaht<T>` 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.
//
// [ii]: http://smallcultfollowing.com/babysteps/blog/2016/09/24/intersection-impls/
#![feature(rustc_attrs, never_type)]
trait MyTrait {}
impl MyTrait for ! {}
trait MyFrom<T> {
fn my_from(x: T) -> Self;
}
// Given the "normal" impls for From
#[rustc_reservation_impl="this impl is reserved"]
impl<T> MyFrom<!> for T {
fn my_from(x: !) -> Self { match x {} }
}
impl<T> MyFrom<T> for T {
fn my_from(x: T) -> Self { x }
}
// ... we *do* want to allow this common pattern, of `From<!> for MySmaht<T>`
struct MySmaht<T>(T);
impl<T> MyFrom<T> for MySmaht<T> {
fn my_from(x: T) -> Self { MySmaht(x) }
}
fn main() {}

View File

@ -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<S> {
fn foo(&self, s: S) -> usize;
}
#[rustc_reservation_impl = "foo"]
impl<T> MyTrait<u64> for T {
fn foo(&self, _x: u64) -> usize { 0 }
}
// reservation impls don't create coherence conflicts, even with
// non-chain overlap.
impl<S> MyTrait<S> for u32 {
fn foo(&self, _x: S) -> usize { mem::size_of::<S>() }
}
fn main() {
// ...and the non-reservation impl gets picked.XS
assert_eq!(0u32.foo(0u64), mem::size_of::<u64>());
}