From 39b79285bef8d1c1a9bea7e2a5f48fc1f92797f5 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Wed, 15 Apr 2015 06:11:18 -0400 Subject: [PATCH] Augment the constrainted parameter check to ensure that all regions 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. --- src/librustc_typeck/collect.rs | 78 ++++++++++++++++--- .../impl-unused-rps-in-assoc-type.rs | 28 +++++++ src/test/compile-fail/issue-22886.rs | 31 ++++++++ 3 files changed, 127 insertions(+), 10 deletions(-) create mode 100644 src/test/compile-fail/impl-unused-rps-in-assoc-type.rs create mode 100644 src/test/compile-fail/issue-22886.rs diff --git a/src/librustc_typeck/collect.rs b/src/librustc_typeck/collect.rs index dfcf375bdbc5f..5ed93703d977f 100644 --- a/src/librustc_typeck/collect.rs +++ b/src/librustc_typeck/collect.rs @@ -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); @@ -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]) { let impl_scheme = ty::lookup_item_type(tcx, impl_def_id); let impl_predicates = ty::lookup_predicates(tcx, impl_def_id); @@ -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", ¶m_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(®ion) && // (*) + !input_parameters.contains(&ctp::Parameter::Region(region)) + { + report_unused_parameter(tcx, lifetime_def.lifetime.span, + "lifetime", ®ion.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 { + // .... + // } + // } + // } + // ``` + // + // 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); } diff --git a/src/test/compile-fail/impl-unused-rps-in-assoc-type.rs b/src/test/compile-fail/impl-unused-rps-in-assoc-type.rs new file mode 100644 index 0000000000000..23401db21d890 --- /dev/null +++ b/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 or the MIT license +// , 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() { } diff --git a/src/test/compile-fail/issue-22886.rs b/src/test/compile-fail/issue-22886.rs new file mode 100644 index 0000000000000..4aa2571cad0cc --- /dev/null +++ b/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 or the MIT license +// , 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>); + +impl<'a> Iterator for Newtype { //~ ERROR E0207 + type Item = &'a Box; + + fn next(&mut self) -> Option<&Box> { + self.0.as_ref() + } +} + +fn main() { }