Skip to content

Deny specializing items not in the parent impl #64564

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Oct 6, 2019
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 24 additions & 2 deletions src/liballoc/boxed.rs
Original file line number Diff line number Diff line change
@@ -871,11 +871,33 @@ impl<I: Iterator + ?Sized> Iterator for Box<I> {
fn nth(&mut self, n: usize) -> Option<I::Item> {
(**self).nth(n)
}
fn last(self) -> Option<I::Item> {
BoxIter::last(self)
}
}

trait BoxIter {
type Item;
fn last(self) -> Option<Self::Item>;
}

impl<I: Iterator + ?Sized> BoxIter for Box<I> {
type Item = I::Item;
default fn last(self) -> Option<I::Item> {
#[inline]
fn some<T>(_: Option<T>, x: T) -> Option<T> {
Some(x)
}

self.fold(None, some)
}
}

/// Specialization for sized `I`s that uses `I`s implementation of `last()`
/// instead of the default.
#[stable(feature = "rust1", since = "1.0.0")]
impl<I: Iterator + Sized> Iterator for Box<I> {
fn last(self) -> Option<I::Item> where I: Sized {
impl<I: Iterator> BoxIter for Box<I> {
fn last(self) -> Option<I::Item> {
(*self).last()
}
}
4 changes: 2 additions & 2 deletions src/librustc/traits/project.rs
Original file line number Diff line number Diff line change
@@ -1505,8 +1505,8 @@ fn assoc_ty_def(

if let Some(assoc_item) = trait_def
.ancestors(tcx, impl_def_id)
.defs(tcx, assoc_ty_name, ty::AssocKind::Type, trait_def_id)
.next() {
.leaf_def(tcx, assoc_ty_name, ty::AssocKind::Type) {

assoc_item
} else {
// This is saying that neither the trait nor
2 changes: 1 addition & 1 deletion src/librustc/traits/specialize/mod.rs
Original file line number Diff line number Diff line change
@@ -125,7 +125,7 @@ pub fn find_associated_item<'tcx>(
let trait_def = tcx.trait_def(trait_def_id);

let ancestors = trait_def.ancestors(tcx, impl_data.impl_def_id);
match ancestors.defs(tcx, item.ident, item.kind, trait_def_id).next() {
match ancestors.leaf_def(tcx, item.ident, item.kind) {
Some(node_item) => {
let substs = tcx.infer_ctxt().enter(|infcx| {
let param_env = param_env.with_reveal_all();
63 changes: 39 additions & 24 deletions src/librustc/traits/specialize/specialization_graph.rs
Original file line number Diff line number Diff line change
@@ -7,7 +7,6 @@ use crate::traits;
use crate::ty::{self, TyCtxt, TypeFoldable};
use crate::ty::fast_reject::{self, SimplifiedType};
use syntax::ast::Ident;
use crate::util::captures::Captures;
use crate::util::nodemap::{DefIdMap, FxHashMap};

/// A per-trait graph of impls in specialization order. At the moment, this
@@ -419,6 +418,35 @@ impl<'tcx> Node {
tcx.associated_items(self.def_id())
}

/// Finds an associated item defined in this node.
///
/// If this returns `None`, the item can potentially still be found in
/// parents of this node.
pub fn item(
&self,
tcx: TyCtxt<'tcx>,
trait_item_name: Ident,
trait_item_kind: ty::AssocKind,
trait_def_id: DefId,
) -> Option<ty::AssocItem> {
use crate::ty::AssocKind::*;

tcx.associated_items(self.def_id())
.find(move |impl_item| match (trait_item_kind, impl_item.kind) {
| (Const, Const)
| (Method, Method)
| (Type, Type)
| (Type, OpaqueTy) // assoc. types can be made opaque in impls
=> tcx.hygienic_eq(impl_item.ident, trait_item_name, trait_def_id),

| (Const, _)
| (Method, _)
| (Type, _)
| (OpaqueTy, _)
=> false,
})
}

pub fn def_id(&self) -> DefId {
match *self {
Node::Impl(did) => did,
@@ -427,6 +455,7 @@ impl<'tcx> Node {
}
}

#[derive(Copy, Clone)]
pub struct Ancestors<'tcx> {
trait_def_id: DefId,
specialization_graph: &'tcx Graph,
@@ -465,32 +494,18 @@ impl<T> NodeItem<T> {
}

impl<'tcx> Ancestors<'tcx> {
/// Search the items from the given ancestors, returning each definition
/// with the given name and the given kind.
// FIXME(#35870): avoid closures being unexported due to `impl Trait`.
#[inline]
pub fn defs(
self,
/// Finds the bottom-most (ie. most specialized) definition of an associated
/// item.
pub fn leaf_def(
mut self,
tcx: TyCtxt<'tcx>,
trait_item_name: Ident,
trait_item_kind: ty::AssocKind,
trait_def_id: DefId,
) -> impl Iterator<Item = NodeItem<ty::AssocItem>> + Captures<'tcx> + 'tcx {
self.flat_map(move |node| {
use crate::ty::AssocKind::*;
node.items(tcx).filter(move |impl_item| match (trait_item_kind, impl_item.kind) {
| (Const, Const)
| (Method, Method)
| (Type, Type)
| (Type, OpaqueTy)
=> tcx.hygienic_eq(impl_item.ident, trait_item_name, trait_def_id),

| (Const, _)
| (Method, _)
| (Type, _)
| (OpaqueTy, _)
=> false,
}).map(move |item| NodeItem { node: node, item: item })
) -> Option<NodeItem<ty::AssocItem>> {
let trait_def_id = self.trait_def_id;
self.find_map(|node| {
node.item(tcx, trait_item_name, trait_item_kind, trait_def_id)
.map(|item| NodeItem { node, item })
})
}
}
5 changes: 2 additions & 3 deletions src/librustc/traits/util.rs
Original file line number Diff line number Diff line change
@@ -4,7 +4,6 @@ use syntax_pos::Span;

use crate::hir;
use crate::hir::def_id::DefId;
use crate::traits::specialize::specialization_graph::NodeItem;
use crate::ty::{self, Ty, TyCtxt, ToPredicate, ToPolyTraitRef};
use crate::ty::outlives::Component;
use crate::ty::subst::{GenericArg, Subst, SubstsRef};
@@ -667,8 +666,8 @@ impl<'tcx> TyCtxt<'tcx> {
}
}

pub fn impl_item_is_final(self, node_item: &NodeItem<hir::Defaultness>) -> bool {
node_item.item.is_final() && !self.impl_is_default(node_item.node.def_id())
pub fn impl_item_is_final(self, assoc_item: &ty::AssocItem) -> bool {
assoc_item.defaultness.is_final() && !self.impl_is_default(assoc_item.container.id())
}
}

11 changes: 11 additions & 0 deletions src/librustc/ty/query/config.rs
Original file line number Diff line number Diff line change
@@ -73,6 +73,17 @@ impl<'tcx, M: QueryAccessors<'tcx, Key = DefId>> QueryDescription<'tcx> for M {
format!("processing {:?} with query `{}`", def_id, name).into()
}
}

default fn cache_on_disk(_: TyCtxt<'tcx>, _: Self::Key, _: Option<&Self::Value>) -> bool {
false
}

default fn try_load_from_disk(
_: TyCtxt<'tcx>,
_: SerializedDepNodeIndex,
) -> Option<Self::Value> {
bug!("QueryDescription::load_from_disk() called for an unsupported query.")
}
}

impl<'tcx> QueryDescription<'tcx> for queries::analysis<'tcx> {
55 changes: 45 additions & 10 deletions src/librustc_typeck/check/mod.rs
Original file line number Diff line number Diff line change
@@ -1713,24 +1713,60 @@ fn check_specialization_validity<'tcx>(
impl_id: DefId,
impl_item: &hir::ImplItem,
) {
let ancestors = trait_def.ancestors(tcx, impl_id);

let kind = match impl_item.kind {
hir::ImplItemKind::Const(..) => ty::AssocKind::Const,
hir::ImplItemKind::Method(..) => ty::AssocKind::Method,
hir::ImplItemKind::OpaqueTy(..) => ty::AssocKind::OpaqueTy,
hir::ImplItemKind::TyAlias(_) => ty::AssocKind::Type,
};

let parent = ancestors.defs(tcx, trait_item.ident, kind, trait_def.def_id).nth(1)
.map(|node_item| node_item.map(|parent| parent.defaultness));
let mut ancestor_impls = trait_def.ancestors(tcx, impl_id)
.skip(1)
.filter_map(|parent| {
if parent.is_from_trait() {
None
} else {
Some((parent, parent.item(tcx, trait_item.ident, kind, trait_def.def_id)))
}
})
.peekable();

if let Some(parent) = parent {
if tcx.impl_item_is_final(&parent) {
report_forbidden_specialization(tcx, impl_item, parent.node.def_id());
}
if ancestor_impls.peek().is_none() {
// No parent, nothing to specialize.
return;
}

let opt_result = ancestor_impls.find_map(|(parent_impl, parent_item)| {
match parent_item {
// Parent impl exists, and contains the parent item we're trying to specialize, but
// doesn't mark it `default`.
Some(parent_item) if tcx.impl_item_is_final(&parent_item) => {
Some(Err(parent_impl.def_id()))
}

// Parent impl contains item and makes it specializable.
Some(_) => {
Some(Ok(()))
}

// Parent impl doesn't mention the item. This means it's inherited from the
// grandparent. In that case, if parent is a `default impl`, inherited items use the
// "defaultness" from the grandparent, else they are final.
None => if tcx.impl_is_default(parent_impl.def_id()) {
None
} else {
Some(Err(parent_impl.def_id()))
}
}
});

// If `opt_result` is `None`, we have only encoutered `default impl`s that don't contain the
// item. This is allowed, the item isn't actually getting specialized here.
let result = opt_result.unwrap_or(Ok(()));

if let Err(parent_impl) = result {
report_forbidden_specialization(tcx, impl_item, parent_impl);
}
}

fn check_impl_items_against_trait<'tcx>(
@@ -1846,8 +1882,7 @@ fn check_impl_items_against_trait<'tcx>(
let associated_type_overridden = overridden_associated_type.is_some();
for trait_item in tcx.associated_items(impl_trait_ref.def_id) {
let is_implemented = trait_def.ancestors(tcx, impl_id)
.defs(tcx, trait_item.ident, trait_item.kind, impl_trait_ref.def_id)
.next()
.leaf_def(tcx, trait_item.ident, trait_item.kind)
.map(|node_item| !node_item.node.is_from_trait())
.unwrap_or(false);

Original file line number Diff line number Diff line change
@@ -22,7 +22,9 @@ pub trait Bar {
fn bar(&self) -> i32 { 0 }
}

impl<T> Bar for T {} // use the provided method
impl<T> Bar for T {
default fn bar(&self) -> i32 { 0 }
}

impl Bar for i32 {
fn bar(&self) -> i32 { 1 }
4 changes: 4 additions & 0 deletions src/test/ui/specialization/issue-36804.rs
Original file line number Diff line number Diff line change
@@ -13,6 +13,10 @@ where
fn next(&mut self) -> Option<T> {
unimplemented!()
}

default fn count(self) -> usize where Self: Sized {
self.fold(0, |cnt, _| cnt + 1)
}
}

impl<'a, I, T: 'a> Iterator for Cloned<I>
53 changes: 53 additions & 0 deletions src/test/ui/specialization/non-defaulted-item-fail.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
#![feature(specialization, associated_type_defaults)]

// Test that attempting to override a non-default method or one not in the
// parent impl causes an error.

trait Foo {
type Ty = ();
const CONST: u8 = 123;
fn foo(&self) -> bool { true }
}

// Specialization tree for Foo:
//
// Box<T> Vec<T>
// / \ / \
// Box<i32> Box<i64> Vec<()> Vec<bool>

impl<T> Foo for Box<T> {
type Ty = bool;
const CONST: u8 = 0;
fn foo(&self) -> bool { false }
}

// Allowed
impl Foo for Box<i32> {}

// Can't override a non-`default` fn
impl Foo for Box<i64> {
type Ty = Vec<()>;
//~^ error: `Ty` specializes an item from a parent `impl`, but that item is not marked `default`
const CONST: u8 = 42;
//~^ error: `CONST` specializes an item from a parent `impl`, but that item is not marked `default`
fn foo(&self) -> bool { true }
//~^ error: `foo` specializes an item from a parent `impl`, but that item is not marked `default`
}


// Doesn't mention the item = provided body/value is used and the method is final.
impl<T> Foo for Vec<T> {}

// Allowed
impl Foo for Vec<()> {}

impl Foo for Vec<bool> {
type Ty = Vec<()>;
//~^ error: `Ty` specializes an item from a parent `impl`, but that item is not marked `default`
const CONST: u8 = 42;
//~^ error: `CONST` specializes an item from a parent `impl`, but that item is not marked `default`
fn foo(&self) -> bool { true }
//~^ error: `foo` specializes an item from a parent `impl`, but that item is not marked `default`
}

fn main() {}
81 changes: 81 additions & 0 deletions src/test/ui/specialization/non-defaulted-item-fail.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
error[E0520]: `Ty` specializes an item from a parent `impl`, but that item is not marked `default`
--> $DIR/non-defaulted-item-fail.rs:29:5
|
LL | / impl<T> Foo for Box<T> {
LL | | type Ty = bool;
LL | | const CONST: u8 = 0;
LL | | fn foo(&self) -> bool { false }
LL | | }
| |_- parent `impl` is here
...
LL | type Ty = Vec<()>;
| ^^^^^^^^^^^^^^^^^^ cannot specialize default item `Ty`
|
= note: to specialize, `Ty` in the parent `impl` must be marked `default`

error[E0520]: `CONST` specializes an item from a parent `impl`, but that item is not marked `default`
--> $DIR/non-defaulted-item-fail.rs:31:5
|
LL | / impl<T> Foo for Box<T> {
LL | | type Ty = bool;
LL | | const CONST: u8 = 0;
LL | | fn foo(&self) -> bool { false }
LL | | }
| |_- parent `impl` is here
...
LL | const CONST: u8 = 42;
| ^^^^^^^^^^^^^^^^^^^^^ cannot specialize default item `CONST`
|
= note: to specialize, `CONST` in the parent `impl` must be marked `default`

error[E0520]: `foo` specializes an item from a parent `impl`, but that item is not marked `default`
--> $DIR/non-defaulted-item-fail.rs:33:5
|
LL | / impl<T> Foo for Box<T> {
LL | | type Ty = bool;
LL | | const CONST: u8 = 0;
LL | | fn foo(&self) -> bool { false }
LL | | }
| |_- parent `impl` is here
...
LL | fn foo(&self) -> bool { true }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ cannot specialize default item `foo`
|
= note: to specialize, `foo` in the parent `impl` must be marked `default`

error[E0520]: `Ty` specializes an item from a parent `impl`, but that item is not marked `default`
--> $DIR/non-defaulted-item-fail.rs:45:5
|
LL | impl<T> Foo for Vec<T> {}
| ------------------------- parent `impl` is here
...
LL | type Ty = Vec<()>;
| ^^^^^^^^^^^^^^^^^^ cannot specialize default item `Ty`
|
= note: to specialize, `Ty` in the parent `impl` must be marked `default`

error[E0520]: `CONST` specializes an item from a parent `impl`, but that item is not marked `default`
--> $DIR/non-defaulted-item-fail.rs:47:5
|
LL | impl<T> Foo for Vec<T> {}
| ------------------------- parent `impl` is here
...
LL | const CONST: u8 = 42;
| ^^^^^^^^^^^^^^^^^^^^^ cannot specialize default item `CONST`
|
= note: to specialize, `CONST` in the parent `impl` must be marked `default`

error[E0520]: `foo` specializes an item from a parent `impl`, but that item is not marked `default`
--> $DIR/non-defaulted-item-fail.rs:49:5
|
LL | impl<T> Foo for Vec<T> {}
| ------------------------- parent `impl` is here
...
LL | fn foo(&self) -> bool { true }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ cannot specialize default item `foo`
|
= note: to specialize, `foo` in the parent `impl` must be marked `default`

error: aborting due to 6 previous errors

For more information about this error, try `rustc --explain E0520`.
5 changes: 3 additions & 2 deletions src/test/ui/specialization/specialization-default-methods.rs
Original file line number Diff line number Diff line change
@@ -55,8 +55,9 @@ trait Bar {
// / \
// Vec<i32> $Vec<i64>

// use the provided method
impl<T> Bar for T {}
impl<T> Bar for T {
default fn bar(&self) -> i32 { 0 }
}

impl Bar for i32 {
fn bar(&self) -> i32 { 1 }