Skip to content

Commit

Permalink
Implement impl reachability rules. This is a [breaking-change]. Type
Browse files Browse the repository at this point in the history
parameters on impls must now also appear in the trait ref, self type,
or some associated type declared on the impl. This ensures that they
are constrianed in some way and that the semantics of the trait system
are well-defined (always a good thing).

There are three major ways to fix this error:

1. Convert the trait to use associated types; most often the type
   parameters are not constrained because they are in fact outputs of
   the impl.

2. Move the type parameters to methods.

3. Add an additional type parameter to the self type or trait so that
   the unused parameter can appear there.

In some cases, it is not possible to fix the impl because the trait
definition needs to be changed first (and that may be out of your
control). In that case, for the time being, you can opt out of these
rules by using `#[old_impl_check]` on the impl and adding a
`#![feature(old_impl_check)]` to your crate declaration.
  • Loading branch information
nikomatsakis committed Jan 6, 2015
1 parent 3ed7f06 commit 2375a79
Show file tree
Hide file tree
Showing 3 changed files with 110 additions and 1 deletion.
1 change: 1 addition & 0 deletions src/librustc/lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -669,6 +669,7 @@ impl LintPass for UnusedAttributes {

// FIXME: #19470 this shouldn't be needed forever
"old_orphan_check",
"old_impl_check",
];

static CRATE_ATTRS: &'static [&'static str] = &[
Expand Down
100 changes: 99 additions & 1 deletion src/librustc_typeck/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ use middle::lang_items::SizedTraitLangItem;
use middle::region;
use middle::resolve_lifetime;
use middle::subst;
use middle::subst::{Substs};
use middle::subst::{Substs, TypeSpace};
use middle::ty::{AsPredicate, ImplContainer, ImplOrTraitItemContainer, TraitContainer};
use middle::ty::{self, RegionEscape, Ty, TypeScheme};
use middle::ty_fold::{self, TypeFolder, TypeFoldable};
Expand All @@ -47,6 +47,7 @@ use util::ppaux;
use util::ppaux::{Repr,UserString};
use write_ty_to_tcx;

use std::collections::HashSet;
use std::rc::Rc;

use syntax::abi;
Expand Down Expand Up @@ -644,6 +645,10 @@ fn convert(ccx: &CollectCtxt, it: &ast::Item) {
Some(selfty),
None);
}

enforce_impl_ty_params_are_constrained(ccx.tcx,
generics,
local_def(it.id));
},
ast::ItemTrait(_, _, _, ref trait_methods) => {
let trait_def = trait_def_of_item(ccx, it);
Expand Down Expand Up @@ -1605,3 +1610,96 @@ fn check_method_self_type<'a, 'tcx, RS:RegionScope>(
})
}
}

/// Checks that all the type parameters on an impl
fn enforce_impl_ty_params_are_constrained<'tcx>(tcx: &ty::ctxt<'tcx>,
ast_generics: &ast::Generics,
impl_def_id: ast::DefId)
{
let impl_scheme = ty::lookup_item_type(tcx, impl_def_id);
let impl_trait_ref = ty::impl_trait_ref(tcx, impl_def_id);

// The trait reference is an input, so find all type parameters
// reachable from there, to start (if this is an inherent impl,
// then just examine the self type).
let mut input_parameters: HashSet<_> =
impl_trait_ref.iter()
.flat_map(|t| t.input_types().iter()) // Types in trait ref, if any
.chain(Some(impl_scheme.ty).iter()) // Self type, always
.flat_map(|t| t.walk())
.filter_map(to_opt_param_ty)
.collect();

loop {
let num_inputs = input_parameters.len();

let mut projection_predicates =
impl_scheme.generics.predicates
.iter()
.filter_map(|predicate| {
match *predicate {
// Ignore higher-ranked binders. For the purposes
// of this check, they don't matter because they
// only affect named regions, and we're just
// concerned about type parameters here.
ty::Predicate::Projection(ref data) => Some(data.0.clone()),
_ => None,
}
});

for projection in projection_predicates {
// Special case: watch out for some kind of sneaky attempt
// to project out an associated type defined by this very trait.
if Some(projection.projection_ty.trait_ref.clone()) == impl_trait_ref {
continue;
}

let relies_only_on_inputs =
projection.projection_ty.trait_ref.input_types().iter()
.flat_map(|t| t.walk())
.filter_map(to_opt_param_ty)
.all(|t| input_parameters.contains(&t));

if relies_only_on_inputs {
input_parameters.extend(
projection.ty.walk().filter_map(to_opt_param_ty));
}
}

if input_parameters.len() == num_inputs {
break;
}
}

for (index, ty_param) in ast_generics.ty_params.iter().enumerate() {
let param_ty = ty::ParamTy { space: TypeSpace,
idx: index as u32,
name: ty_param.ident.name };
if !input_parameters.contains(&param_ty) {
if ty::has_attr(tcx, impl_def_id, "old_impl_check") {
tcx.sess.span_warn(
ty_param.span,
format!("the type parameter `{}` is not constrained by the \
impl trait, self type, or predicates",
param_ty.user_string(tcx)).as_slice());
} else {
tcx.sess.span_err(
ty_param.span,
format!("the type parameter `{}` is not constrained by the \
impl trait, self type, or predicates",
param_ty.user_string(tcx)).as_slice());
tcx.sess.span_help(
ty_param.span,
format!("you can temporarily opt out of this rule by placing \
the `#[old_impl_check]` attribute on the impl").as_slice());
}
}
}

fn to_opt_param_ty<'tcx>(ty: Ty<'tcx>) -> Option<ty::ParamTy> {
match ty.sty {
ty::ty_param(ref d) => Some(d.clone()),
_ => None,
}
}
}
10 changes: 10 additions & 0 deletions src/libsyntax/feature_gate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,9 @@ static KNOWN_FEATURES: &'static [(&'static str, Status)] = &[
// A way to temporarily opt out of the new orphan rules. This will *never* be accepted.
("old_orphan_check", Deprecated),

// A way to temporarily opt out of the new impl rules. This will *never* be accepted.
("old_impl_check", Deprecated),

// OIBIT specific features
("optin_builtin_traits", Active),

Expand Down Expand Up @@ -294,6 +297,13 @@ impl<'a, 'v> Visitor<'v> for PostExpansionVisitor<'a> {
i.span,
"the new orphan check rules will eventually be strictly enforced");
}

if attr::contains_name(i.attrs[],
"old_impl_check") {
self.gate_feature("old_impl_check",
i.span,
"`#[old_impl_check]` will be removed in the future");
}
}

_ => {}
Expand Down

0 comments on commit 2375a79

Please sign in to comment.