Skip to content

#37653 support default impl for specialization #37860

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
Apr 27, 2017
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
21 changes: 14 additions & 7 deletions src/grammar/parser-lalr.y
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ extern char *yytext;
%token TRAIT
%token TYPE
%token UNSAFE
%token DEFAULT
%token USE
%token WHILE
%token CONTINUE
Expand Down Expand Up @@ -534,6 +535,12 @@ maybe_unsafe
| %empty { $$ = mk_none(); }
;

maybe_default_maybe_unsafe
: DEFAULT UNSAFE { $$ = mk_atom("DefaultUnsafe"); }
| DEFAULT { $$ = mk_atom("Default"); }
| UNSAFE { $$ = mk_atom("Unsafe"); }
| %empty { $$ = mk_none(); }

trait_method
: type_method { $$ = mk_node("Required", 1, $1); }
| method { $$ = mk_node("Provided", 1, $1); }
Expand Down Expand Up @@ -588,27 +595,27 @@ impl_method
// they are ambiguous with traits. We do the same here, regrettably,
// by splitting ty into ty and ty_prim.
item_impl
: maybe_unsafe IMPL generic_params ty_prim_sum maybe_where_clause '{' maybe_inner_attrs maybe_impl_items '}'
: maybe_default_maybe_unsafe IMPL generic_params ty_prim_sum maybe_where_clause '{' maybe_inner_attrs maybe_impl_items '}'
{
$$ = mk_node("ItemImpl", 6, $1, $3, $4, $5, $7, $8);
}
| maybe_unsafe IMPL generic_params '(' ty ')' maybe_where_clause '{' maybe_inner_attrs maybe_impl_items '}'
| maybe_default_maybe_unsafe IMPL generic_params '(' ty ')' maybe_where_clause '{' maybe_inner_attrs maybe_impl_items '}'
{
$$ = mk_node("ItemImpl", 6, $1, $3, 5, $6, $9, $10);
}
| maybe_unsafe IMPL generic_params trait_ref FOR ty_sum maybe_where_clause '{' maybe_inner_attrs maybe_impl_items '}'
| maybe_default_maybe_unsafe IMPL generic_params trait_ref FOR ty_sum maybe_where_clause '{' maybe_inner_attrs maybe_impl_items '}'
{
$$ = mk_node("ItemImpl", 6, $3, $4, $6, $7, $9, $10);
}
| maybe_unsafe IMPL generic_params '!' trait_ref FOR ty_sum maybe_where_clause '{' maybe_inner_attrs maybe_impl_items '}'
| maybe_default_maybe_unsafe IMPL generic_params '!' trait_ref FOR ty_sum maybe_where_clause '{' maybe_inner_attrs maybe_impl_items '}'
{
$$ = mk_node("ItemImplNeg", 7, $1, $3, $5, $7, $8, $10, $11);
}
| maybe_unsafe IMPL generic_params trait_ref FOR DOTDOT '{' '}'
| maybe_default_maybe_unsafe IMPL generic_params trait_ref FOR DOTDOT '{' '}'
{
$$ = mk_node("ItemImplDefault", 3, $1, $3, $4);
}
| maybe_unsafe IMPL generic_params '!' trait_ref FOR DOTDOT '{' '}'
| maybe_default_maybe_unsafe IMPL generic_params '!' trait_ref FOR DOTDOT '{' '}'
{
$$ = mk_node("ItemImplDefaultNeg", 3, $1, $3, $4);
}
Expand Down Expand Up @@ -1935,4 +1942,4 @@ brackets_delimited_token_trees
$2,
mk_node("TTTok", 1, mk_atom("]")));
}
;
;
12 changes: 11 additions & 1 deletion src/librustc/hir/lowering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1326,7 +1326,13 @@ impl<'a> LoweringContext<'a> {
hir::ItemDefaultImpl(self.lower_unsafety(unsafety),
trait_ref)
}
ItemKind::Impl(unsafety, polarity, ref generics, ref ifce, ref ty, ref impl_items) => {
ItemKind::Impl(unsafety,
polarity,
defaultness,
ref generics,
ref ifce,
ref ty,
ref impl_items) => {
let new_impl_items = impl_items.iter()
.map(|item| self.lower_impl_item_ref(item))
.collect();
Expand All @@ -1340,6 +1346,7 @@ impl<'a> LoweringContext<'a> {

hir::ItemImpl(self.lower_unsafety(unsafety),
self.lower_impl_polarity(polarity),
self.lower_defaultness(defaultness, true /* [1] */),
self.lower_generics(generics),
ifce,
self.lower_ty(ty),
Expand All @@ -1355,6 +1362,9 @@ impl<'a> LoweringContext<'a> {
}
ItemKind::MacroDef(..) | ItemKind::Mac(..) => panic!("Shouldn't still be around"),
}

// [1] `defaultness.has_value()` is never called for an `impl`, always `true` in order to
// not cause an assertion failure inside the `lower_defaultness` function
}

fn lower_trait_item(&mut self, i: &TraitItem) -> hir::TraitItem {
Expand Down
1 change: 1 addition & 0 deletions src/librustc/hir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1712,6 +1712,7 @@ pub enum Item_ {
/// An implementation, eg `impl<A> Trait for Foo { .. }`
ItemImpl(Unsafety,
ImplPolarity,
Defaultness,
Generics,
Option<TraitRef>, // (optional) trait this impl implements
P<Ty>, // self
Expand Down
16 changes: 11 additions & 5 deletions src/librustc/hir/print.rs
Original file line number Diff line number Diff line change
Expand Up @@ -678,12 +678,14 @@ impl<'a> State<'a> {
}
hir::ItemImpl(unsafety,
polarity,
defaultness,
ref generics,
ref opt_trait,
ref ty,
ref impl_items) => {
self.head("")?;
self.print_visibility(&item.vis)?;
self.print_defaultness(defaultness)?;
Copy link
Member

Choose a reason for hiding this comment

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

This has a different order compared to the yacc grammar.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've changed the parser-lalr.y accordingly to the [default] [unsafe] impl order so this file should be ok now

self.print_unsafety(unsafety)?;
self.word_nbsp("impl")?;

Expand Down Expand Up @@ -820,6 +822,14 @@ impl<'a> State<'a> {
}
}

pub fn print_defaultness(&mut self, defaultness: hir::Defaultness) -> io::Result<()> {
match defaultness {
hir::Defaultness::Default { .. } => self.word_nbsp("default")?,
hir::Defaultness::Final => (),
}
Ok(())
}

pub fn print_struct(&mut self,
struct_def: &hir::VariantData,
generics: &hir::Generics,
Expand Down Expand Up @@ -931,11 +941,7 @@ impl<'a> State<'a> {
self.hardbreak_if_not_bol()?;
self.maybe_print_comment(ii.span.lo)?;
self.print_outer_attributes(&ii.attrs)?;

match ii.defaultness {
hir::Defaultness::Default { .. } => self.word_nbsp("default")?,
hir::Defaultness::Final => (),
}
self.print_defaultness(ii.defaultness)?;

match ii.node {
hir::ImplItemKind::Const(ref ty, expr) => {
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/ich/impls_hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -933,7 +933,7 @@ impl_stable_hash_for!(enum hir::Item_ {
ItemUnion(variant_data, generics),
ItemTrait(unsafety, generics, bounds, item_refs),
ItemDefaultImpl(unsafety, trait_ref),
ItemImpl(unsafety, impl_polarity, generics, trait_ref, ty, impl_item_refs)
ItemImpl(unsafety, impl_polarity, impl_defaultness, generics, trait_ref, ty, impl_item_refs)
});

impl_stable_hash_for!(struct hir::TraitItemRef {
Expand Down
2 changes: 2 additions & 0 deletions src/librustc/middle/cstore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,7 @@ pub trait CrateStore {
fn implementations_of_trait(&self, filter: Option<DefId>) -> Vec<DefId>;

// impl info
fn impl_defaultness(&self, def: DefId) -> hir::Defaultness;
fn impl_parent(&self, impl_def_id: DefId) -> Option<DefId>;

// trait/impl-item info
Expand Down Expand Up @@ -329,6 +330,7 @@ impl CrateStore for DummyCrateStore {
fn implementations_of_trait(&self, filter: Option<DefId>) -> Vec<DefId> { vec![] }

// impl info
fn impl_defaultness(&self, def: DefId) -> hir::Defaultness { bug!("impl_defaultness") }
fn impl_parent(&self, def: DefId) -> Option<DefId> { bug!("impl_parent") }

// trait/impl-item info
Expand Down
4 changes: 2 additions & 2 deletions src/librustc/middle/reachable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ fn item_might_be_inlined(item: &hir::Item) -> bool {
}

match item.node {
hir::ItemImpl(_, _, ref generics, ..) |
hir::ItemImpl(_, _, _, ref generics, ..) |
hir::ItemFn(.., ref generics, _) => {
generics_require_inlining(generics)
}
Expand Down Expand Up @@ -186,7 +186,7 @@ impl<'a, 'tcx> ReachableContext<'a, 'tcx> {
// does too.
let impl_node_id = self.tcx.hir.as_local_node_id(impl_did).unwrap();
match self.tcx.hir.expect_item(impl_node_id).node {
hir::ItemImpl(_, _, ref generics, ..) => {
hir::ItemImpl(_, _, _, ref generics, ..) => {
generics_require_inlining(generics)
}
_ => false
Expand Down
4 changes: 2 additions & 2 deletions src/librustc/middle/resolve_lifetime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,7 @@ impl<'a, 'tcx> Visitor<'tcx> for LifetimeContext<'a, 'tcx> {
hir::ItemStruct(_, ref generics) |
hir::ItemUnion(_, ref generics) |
hir::ItemTrait(_, ref generics, ..) |
hir::ItemImpl(_, _, ref generics, ..) => {
hir::ItemImpl(_, _, _, ref generics, ..) => {
// These kinds of items have only early bound lifetime parameters.
let mut index = if let hir::ItemTrait(..) = item.node {
1 // Self comes before lifetimes
Expand Down Expand Up @@ -834,7 +834,7 @@ impl<'a, 'tcx> LifetimeContext<'a, 'tcx> {
}
match parent.node {
hir::ItemTrait(_, ref generics, ..) |
hir::ItemImpl(_, _, ref generics, ..) => {
hir::ItemImpl(_, _, _, ref generics, ..) => {
index += (generics.lifetimes.len() + generics.ty_params.len()) as u32;
}
_ => {}
Expand Down
3 changes: 2 additions & 1 deletion src/librustc/traits/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -923,7 +923,8 @@ fn assemble_candidates_from_impls<'cx, 'gcx, 'tcx>(
// being invoked).
node_item.item.defaultness.has_value()
} else {
node_item.item.defaultness.is_default()
node_item.item.defaultness.is_default() ||
selcx.tcx().impl_is_default(node_item.node.def_id())
};

// Only reveal a specializable default if we're past type-checking
Expand Down
26 changes: 26 additions & 0 deletions src/librustc/traits/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ use ty::subst::{Subst, Substs};
use ty::{self, Ty, TyCtxt, ToPredicate, ToPolyTraitRef};
use ty::outlives::Component;
use util::nodemap::FxHashSet;
use hir::{self};
use traits::specialize::specialization_graph::NodeItem;

use super::{Obligation, ObligationCause, PredicateObligation, SelectionContext, Normalized};

Expand Down Expand Up @@ -504,6 +506,30 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
};
ty::Binder((trait_ref, sig.skip_binder().output()))
}

pub fn impl_is_default(self, node_item_def_id: DefId) -> bool {
match self.hir.as_local_node_id(node_item_def_id) {
Some(node_id) => {
let item = self.hir.expect_item(node_id);
if let hir::ItemImpl(_, _, defaultness, ..) = item.node {
defaultness.is_default()
} else {
false
}
}
None => {
self.global_tcx()
.sess
.cstore
.impl_defaultness(node_item_def_id)
.is_default()
}
}
}

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 enum TupleArgumentsFlag { Yes, No }
6 changes: 6 additions & 0 deletions src/librustc_metadata/cstore_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,12 @@ impl CrateStore for cstore::CStore {
result
}

fn impl_defaultness(&self, def: DefId) -> hir::Defaultness
{
self.dep_graph.read(DepNode::MetaData(def));
self.get_crate_data(def.krate).get_impl_defaultness(def.index)
}

fn impl_parent(&self, impl_def: DefId) -> Option<DefId> {
self.dep_graph.read(DepNode::MetaData(impl_def));
self.get_crate_data(impl_def.krate).get_parent_impl(impl_def.index)
Expand Down
4 changes: 4 additions & 0 deletions src/librustc_metadata/decoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -629,6 +629,10 @@ impl<'a, 'tcx> CrateMetadata {
self.get_impl_data(id).polarity
}

pub fn get_impl_defaultness(&self, id: DefIndex) -> hir::Defaultness {
self.get_impl_data(id).defaultness
}

pub fn get_coerce_unsized_info(&self,
id: DefIndex)
-> Option<ty::adjustment::CoerceUnsizedInfo> {
Expand Down
4 changes: 3 additions & 1 deletion src/librustc_metadata/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -706,14 +706,15 @@ impl<'a, 'b: 'a, 'tcx: 'b> EntryBuilder<'a, 'b, 'tcx> {
hir::ItemDefaultImpl(..) => {
let data = ImplData {
polarity: hir::ImplPolarity::Positive,
defaultness: hir::Defaultness::Final,
Copy link
Member

Choose a reason for hiding this comment

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

Are you sure about this being Final? Does this mean that default impl Trait for .. {} is not possible even with this PR?

Copy link
Contributor Author

@giannicic giannicic Apr 21, 2017

Choose a reason for hiding this comment

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

@nagisa I've read the discussion about UnwindSafe #40628, this PR however is one of a set of tasks described here #37653,
the 3rd

all items in a default impl are (implicitly) default and hence specializable

It doesn't address default trait implementations.
At least another PR must follow in order to cover the other two points.
so we could define better what @abonander says about

re-addressed this in the context of specialization

and add it to the task list.
What do you think?

EDIT: I've placed an error during parsing to avoid default impl Trait for .. {}

Copy link
Member

Choose a reason for hiding this comment

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

I’m not sure this PR is a right place to discuss UnwindUnsafe. If you have any ideas, you should post post them on the UnwindUnsafe issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree. After this PR I'll have a better look at UnwindUnsafe issue

parent_impl: None,
coerce_unsized_info: None,
trait_ref: tcx.impl_trait_ref(def_id).map(|trait_ref| self.lazy(&trait_ref)),
};

EntryKind::DefaultImpl(self.lazy(&data))
}
hir::ItemImpl(_, polarity, ..) => {
hir::ItemImpl(_, polarity, defaultness, ..) => {
let trait_ref = tcx.impl_trait_ref(def_id);
let parent = if let Some(trait_ref) = trait_ref {
let trait_def = tcx.lookup_trait_def(trait_ref.def_id);
Expand All @@ -740,6 +741,7 @@ impl<'a, 'b: 'a, 'tcx: 'b> EntryBuilder<'a, 'b, 'tcx> {

let data = ImplData {
polarity: polarity,
defaultness: defaultness,
parent_impl: parent,
coerce_unsized_info: coerce_unsized_info,
trait_ref: trait_ref.map(|trait_ref| self.lazy(&trait_ref)),
Expand Down
2 changes: 2 additions & 0 deletions src/librustc_metadata/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -406,6 +406,7 @@ impl_stable_hash_for!(struct TraitData<'tcx> {
#[derive(RustcEncodable, RustcDecodable)]
pub struct ImplData<'tcx> {
pub polarity: hir::ImplPolarity,
pub defaultness: hir::Defaultness,
pub parent_impl: Option<DefId>,

/// This is `Some` only for impls of `CoerceUnsized`.
Expand All @@ -415,6 +416,7 @@ pub struct ImplData<'tcx> {

impl_stable_hash_for!(struct ImplData<'tcx> {
polarity,
defaultness,
parent_impl,
coerce_unsized_info,
trait_ref
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_save_analysis/dump_visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,7 @@ impl<'l, 'tcx: 'l, 'll, D: Dump + 'll> DumpVisitor<'l, 'tcx, 'll, D> {
}
None => {
if let Some(NodeItem(item)) = self.tcx.hir.get_if_local(id) {
if let hir::ItemImpl(_, _, _, _, ref ty, _) = item.node {
if let hir::ItemImpl(_, _, _, _, _, ref ty, _) = item.node {
trait_id = self.lookup_def_id(ty.id);
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/librustc_trans/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -880,7 +880,7 @@ impl<'b, 'a, 'v> ItemLikeVisitor<'v> for RootCollector<'b, 'a, 'v> {
let parent_node_id = hir_map.get_parent_node(ii.id);
let is_impl_generic = match hir_map.expect_item(parent_node_id) {
&hir::Item {
node: hir::ItemImpl(_, _, ref generics, ..),
node: hir::ItemImpl(_, _, _, ref generics, ..),
..
} => {
generics.is_type_parameterized()
Expand Down Expand Up @@ -911,6 +911,7 @@ fn create_trans_items_for_default_impls<'a, 'tcx>(scx: &SharedCrateContext<'a, '
let tcx = scx.tcx();
match item.node {
hir::ItemImpl(_,
_,
_,
ref generics,
..,
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 @@ -1141,7 +1141,7 @@ fn check_specialization_validity<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
.map(|node_item| node_item.map(|parent| parent.defaultness));

if let Some(parent) = parent {
if parent.item.is_final() {
if tcx.impl_item_is_final(&parent) {
report_forbidden_specialization(tcx, impl_item, parent.node.def_id());
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/librustc_typeck/check/wfcheck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,11 +105,11 @@ impl<'a, 'gcx> CheckTypeWellFormedVisitor<'a, 'gcx> {
///
/// won't be allowed unless there's an *explicit* implementation of `Send`
/// for `T`
hir::ItemImpl(_, hir::ImplPolarity::Positive, _,
hir::ItemImpl(_, hir::ImplPolarity::Positive, _, _,
ref trait_ref, ref self_ty, _) => {
self.check_impl(item, self_ty, trait_ref);
}
hir::ItemImpl(_, hir::ImplPolarity::Negative, _, Some(_), ..) => {
hir::ItemImpl(_, hir::ImplPolarity::Negative, _, _, Some(_), ..) => {
// FIXME(#27579) what amount of WF checking do we need for neg impls?

let trait_ref = tcx.impl_trait_ref(tcx.hir.local_def_id(item.id)).unwrap();
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_typeck/coherence/unsafety.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ impl<'cx, 'tcx, 'v> ItemLikeVisitor<'v> for UnsafetyChecker<'cx, 'tcx> {
hir::ItemDefaultImpl(unsafety, _) => {
self.check_unsafety_coherence(item, None, unsafety, hir::ImplPolarity::Positive);
}
hir::ItemImpl(unsafety, polarity, ref generics, Some(_), _, _) => {
hir::ItemImpl(unsafety, polarity, _, ref generics, ..) => {
self.check_unsafety_coherence(item, Some(generics), unsafety, polarity);
}
_ => {}
Expand Down
Loading