Skip to content

Commit

Permalink
Augment the constrainted parameter check to ensure that all regions
Browse files Browse the repository at this point in the history
which get mentioned in an associated type are constrained.  Arguably we
should just require that all regions are constrained, but that is more
of a breaking change.
  • Loading branch information
nikomatsakis committed Apr 17, 2015
1 parent ad3cbac commit 39b7928
Show file tree
Hide file tree
Showing 3 changed files with 127 additions and 10 deletions.
78 changes: 68 additions & 10 deletions src/librustc_typeck/collect.rs
Expand Up @@ -902,9 +902,10 @@ fn convert_item(ccx: &CrateCtxt, it: &ast::Item) {
tcx.impl_trait_refs.borrow_mut().insert(it.id, trait_ref);
}

enforce_impl_ty_params_are_constrained(tcx,
generics,
local_def(it.id));
enforce_impl_params_are_constrained(tcx,
generics,
local_def(it.id),
impl_items);
},
ast::ItemTrait(_, _, _, ref trait_items) => {
let trait_def = trait_def_of_item(ccx, it);
Expand Down Expand Up @@ -2188,9 +2189,10 @@ 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)
fn enforce_impl_params_are_constrained<'tcx>(tcx: &ty::ctxt<'tcx>,
ast_generics: &ast::Generics,
impl_def_id: ast::DefId,
impl_items: &[P<ast::ImplItem>])
{
let impl_scheme = ty::lookup_item_type(tcx, impl_def_id);
let impl_predicates = ty::lookup_predicates(tcx, impl_def_id);
Expand All @@ -2215,10 +2217,66 @@ fn enforce_impl_ty_params_are_constrained<'tcx>(tcx: &ty::ctxt<'tcx>,
idx: index as u32,
name: ty_param.ident.name };
if !input_parameters.contains(&ctp::Parameter::Type(param_ty)) {
span_err!(tcx.sess, ty_param.span, E0207,
"the type parameter `{}` is not constrained by the \
impl trait, self type, or predicates",
param_ty.user_string(tcx));
report_unused_parameter(tcx, ty_param.span, "type", &param_ty.user_string(tcx));
}
}

// Every lifetime used in an associated type must be constrained.

let lifetimes_in_associated_types: HashSet<_> =
impl_items.iter()
.filter_map(|item| match item.node {
ast::TypeImplItem(..) => Some(ty::node_id_to_type(tcx, item.id)),
ast::MethodImplItem(..) | ast::MacImplItem(..) => None,
})
.flat_map(|ty| ctp::parameters_for_type(ty).into_iter())
.filter_map(|p| match p {
ctp::Parameter::Type(_) => None,
ctp::Parameter::Region(r) => Some(r),
})
.collect();

for (index, lifetime_def) in ast_generics.lifetimes.iter().enumerate() {
let region = ty::EarlyBoundRegion { param_id: lifetime_def.lifetime.id,
space: TypeSpace,
index: index as u32,
name: lifetime_def.lifetime.name };
if
lifetimes_in_associated_types.contains(&region) && // (*)
!input_parameters.contains(&ctp::Parameter::Region(region))
{
report_unused_parameter(tcx, lifetime_def.lifetime.span,
"lifetime", &region.name.user_string(tcx));
}
}

// (*) This is a horrible concession to reality. I think it'd be
// better to just ban unconstrianed lifetimes outright, but in
// practice people do non-hygenic macros like:
//
// ```
// macro_rules! __impl_slice_eq1 {
// ($Lhs: ty, $Rhs: ty, $Bound: ident) => {
// impl<'a, 'b, A: $Bound, B> PartialEq<$Rhs> for $Lhs where A: PartialEq<B> {
// ....
// }
// }
// }
// ```
//
// In a concession to backwards compatbility, we continue to
// permit those, so long as the lifetimes aren't used in
// associated types. I believe this is sound, because lifetimes
// used elsewhere are not projected back out.
}

fn report_unused_parameter(tcx: &ty::ctxt,
span: Span,
kind: &str,
name: &str)
{
span_err!(tcx.sess, span, E0207,
"the {} parameter `{}` is not constrained by the \
impl trait, self type, or predicates",
kind, name);
}
28 changes: 28 additions & 0 deletions src/test/compile-fail/impl-unused-rps-in-assoc-type.rs
@@ -0,0 +1,28 @@
// 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.

// Test that lifetime parameters must be constrained if they appear in
// an associated type def'n. Issue #22077.

trait Fun {
type Output;
fn call<'x>(&'x self) -> Self::Output;
}

struct Holder { x: String }

impl<'a> Fun for Holder { //~ ERROR E0207
type Output = &'a str;
fn call<'b>(&'b self) -> &'b str {
&self.x[..]
}
}

fn main() { }
31 changes: 31 additions & 0 deletions src/test/compile-fail/issue-22886.rs
@@ -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.

// Regression test for #22886.

fn crash_please() {
let mut iter = Newtype(Some(Box::new(0)));
let saved = iter.next().unwrap();
println!("{}", saved);
iter.0 = None;
println!("{}", saved);
}

struct Newtype(Option<Box<usize>>);

impl<'a> Iterator for Newtype { //~ ERROR E0207
type Item = &'a Box<usize>;

fn next(&mut self) -> Option<&Box<usize>> {
self.0.as_ref()
}
}

fn main() { }

0 comments on commit 39b7928

Please sign in to comment.