Skip to content
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

Deny specializing items not in the parent impl #64564

Merged
merged 8 commits into from
Oct 6, 2019
Merged
Show file tree
Hide file tree
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
Expand Up @@ -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> {
Centril marked this conversation as resolved.
Show resolved Hide resolved
#[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()
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/librustc/traits/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/traits/specialize/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
63 changes: 39 additions & 24 deletions src/librustc/traits/specialize/specialization_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -427,6 +455,7 @@ impl<'tcx> Node {
}
}

#[derive(Copy, Clone)]
pub struct Ancestors<'tcx> {
trait_def_id: DefId,
specialization_graph: &'tcx Graph,
Expand Down Expand Up @@ -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`.
Centril marked this conversation as resolved.
Show resolved Hide resolved
#[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 })
})
}
}
Expand Down
5 changes: 2 additions & 3 deletions src/librustc/traits/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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())
}
}

Expand Down
11 changes: 11 additions & 0 deletions src/librustc/ty/query/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down
55 changes: 45 additions & 10 deletions src/librustc_typeck/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>(
Expand Down Expand Up @@ -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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
4 changes: 4 additions & 0 deletions src/test/ui/specialization/issue-36804.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
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() {}
Loading