Skip to content

#37653 support default impl for specialization #45404

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 5 commits into from
Feb 16, 2018
Merged
Show file tree
Hide file tree
Changes from 4 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
44 changes: 33 additions & 11 deletions src/librustc/traits/select.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1710,22 +1710,44 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> {
{
debug!("assemble_candidates_from_impls(obligation={:?})", obligation);

// Check if default impls should be emitted.
// default impls are emitted if the param_env is refered to a default impl.
// The param_env should contain a Self: Trait<..> predicate in those cases
let self_trait_is_present:Vec<&ty::Predicate<'tcx>> =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I don't think we should have to modify select.rs at all. Instead, what I would expect is for us to modify the set of predicates associated with default impls, analogously to what we do with traits. This would be part of the predicates_of query -- for example, in traits, we insert Self: Trait<...> right here:

// Add in a predicate that `Self:Trait` (where `Trait` is the
// current trait). This is needed for builtin bounds.
predicates.push(trait_ref.to_poly_trait_ref().to_predicate());

I would think that we would want to do the same code in the case of a default impl. In that case, I don't think we should have to modify select.rs at all. The required assumptions will be found in the environment.

obligation.param_env
.caller_bounds
.iter()
.filter(|predicate| {
match **predicate {
ty::Predicate::Trait(ref trait_predicate) => {
trait_predicate.def_id() ==
obligation.predicate.def_id() &&
obligation.predicate.0.trait_ref.self_ty() ==
trait_predicate.skip_binder().self_ty()
}
_ => false
}
}).collect::<Vec<&ty::Predicate<'tcx>>>();

self.tcx().for_each_relevant_impl(
obligation.predicate.def_id(),
obligation.predicate.0.trait_ref.self_ty(),
|impl_def_id| {
self.probe(|this, snapshot| { /* [1] */
match this.match_impl(impl_def_id, obligation, snapshot) {
Ok(skol_map) => {
candidates.vec.push(ImplCandidate(impl_def_id));

// NB: we can safely drop the skol map
// since we are in a probe [1]
mem::drop(skol_map);
if self_trait_is_present.len() > 0 ||
!self.tcx().impl_is_default(impl_def_id) {
self.probe(|this, snapshot| { /* [1] */
match this.match_impl(impl_def_id, obligation, snapshot) {
Ok(skol_map) => {
candidates.vec.push(ImplCandidate(impl_def_id));

// NB: we can safely drop the skol map
// since we are in a probe [1]
mem::drop(skol_map);
}
Err(_) => { }
}
Err(_) => { }
}
});
});
}
}
);

Expand Down
25 changes: 24 additions & 1 deletion src/librustc/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2606,8 +2606,31 @@ fn param_env<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
def_id: DefId)
-> ParamEnv<'tcx> {
// Compute the bounds on Self and the type parameters.
let mut predicates = tcx.predicates_of(def_id);
match tcx.hir.as_local_node_id(def_id)
.and_then(|node_id| tcx.hir.find(node_id))
.and_then(|item| {
match item {
hir::map::NodeItem(..) => {
if tcx.impl_is_default(def_id) {
tcx.impl_trait_ref(def_id)
} else {
None
}
}
_ => None
}
}) {
Some(trait_ref) =>
predicates.predicates
.push(
trait_ref.to_poly_trait_ref()
.to_predicate()
),
None => {}
}

let bounds = tcx.predicates_of(def_id).instantiate_identity(tcx);
let bounds = predicates.instantiate_identity(tcx);
let predicates = bounds.predicates;

// Finally, we have to normalize the bounds in the environment, in
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_typeck/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1395,7 +1395,7 @@ fn check_impl_items_against_trait<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
.map(|node_item| !node_item.node.is_from_trait())
.unwrap_or(false);

if !is_implemented {
if !is_implemented && !tcx.impl_is_default(impl_id) {
if !trait_item.defaultness.has_value() {
missing_items.push(trait_item);
} else if associated_type_overridden {
Expand Down

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be nice to have comments on these tests with what they are testing:


Tests that (a) default impls do not have to supply all items but (b) regular impls do.

#![feature(specialization)]

trait Foo {
fn foo_one(&self) -> &'static str;
fn foo_two(&self) -> &'static str;
}

struct MyStruct;

default impl<T> Foo for T {
fn foo_one(&self) -> &'static str {
"generic"
}
}

impl Foo for MyStruct {}
//~^ ERROR not all trait items implemented, missing: `foo_two` [E0046]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍


fn main() {
println!("{}", MyStruct.foo_one());
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#![feature(specialization)]

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be nice to have comments on these tests with what they are testing:


Tests that (a) default impls do not have to supply all items and (b) a default impl does not count as an impl (in this case, an incomplete default impl).

trait Foo {
fn foo_one(&self) -> &'static str;
fn foo_two(&self) -> &'static str;
}

struct MyStruct;

default impl<T> Foo for T {
fn foo_one(&self) -> &'static str {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

"generic"
}
}


fn main() {
println!("{}", MyStruct.foo_one());
//~^ ERROR no method named `foo_one` found for type `MyStruct` in the current scope
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
Expand All @@ -10,23 +10,9 @@

#![feature(specialization)]

// Regression test for ICE when combining specialized associated types and type
// aliases
trait Foo<'a, T: Eq + 'a> { }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment:


Tests that a default impl still has to have a WF trait ref.


trait Id_ {
type Out;
}
default impl<U> Foo<'static, U> for () {}
//~^ ERROR the trait bound `U: std::cmp::Eq` is not satisfied

type Id<T> = <T as Id_>::Out;

default impl<T> Id_ for T {
type Out = T;
}

fn test_proection() {
let x: Id<bool> = panic!();
}

fn main() {

}
fn main(){}

This file was deleted.

Loading