Skip to content

Commit 8b1dafd

Browse files
nikomatsakisalexcrichton
authored andcommitted
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. Conflicts: src/librustc_typeck/collect.rs
1 parent 104bd1d commit 8b1dafd

File tree

3 files changed

+128
-11
lines changed

3 files changed

+128
-11
lines changed

src/librustc_typeck/collect.rs

Lines changed: 69 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -902,9 +902,10 @@ fn convert_item(ccx: &CrateCtxt, it: &ast::Item) {
902902
tcx.impl_trait_refs.borrow_mut().insert(it.id, trait_ref);
903903
}
904904

905-
enforce_impl_ty_params_are_constrained(tcx,
906-
generics,
907-
local_def(it.id));
905+
enforce_impl_params_are_constrained(tcx,
906+
generics,
907+
local_def(it.id),
908+
impl_items);
908909
},
909910
ast::ItemTrait(_, _, _, ref trait_items) => {
910911
let trait_def = trait_def_of_item(ccx, it);
@@ -2188,9 +2189,10 @@ fn check_method_self_type<'a, 'tcx, RS:RegionScope>(
21882189
}
21892190

21902191
/// Checks that all the type parameters on an impl
2191-
fn enforce_impl_ty_params_are_constrained<'tcx>(tcx: &ty::ctxt<'tcx>,
2192-
ast_generics: &ast::Generics,
2193-
impl_def_id: ast::DefId)
2192+
fn enforce_impl_params_are_constrained<'tcx>(tcx: &ty::ctxt<'tcx>,
2193+
ast_generics: &ast::Generics,
2194+
impl_def_id: ast::DefId,
2195+
impl_items: &[P<ast::ImplItem>])
21942196
{
21952197
let impl_scheme = ty::lookup_item_type(tcx, impl_def_id);
21962198
let impl_predicates = ty::lookup_predicates(tcx, impl_def_id);
@@ -2216,11 +2218,67 @@ fn enforce_impl_ty_params_are_constrained<'tcx>(tcx: &ty::ctxt<'tcx>,
22162218
let param_ty = ty::ParamTy { space: TypeSpace,
22172219
idx: index as u32,
22182220
name: ty_param.ident.name };
2219-
if !input_parameters.contains(&param_ty) {
2220-
span_err!(tcx.sess, ty_param.span, E0207,
2221-
"the type parameter `{}` is not constrained by the \
2222-
impl trait, self type, or predicates",
2223-
param_ty.user_string(tcx));
2221+
if !input_parameters.contains(&ctp::Parameter::Type(param_ty)) {
2222+
report_unused_parameter(tcx, ty_param.span, "type", &param_ty.user_string(tcx));
2223+
}
2224+
}
2225+
2226+
// Every lifetime used in an associated type must be constrained.
2227+
2228+
let lifetimes_in_associated_types: HashSet<_> =
2229+
impl_items.iter()
2230+
.filter_map(|item| match item.node {
2231+
ast::TypeImplItem(..) => Some(ty::node_id_to_type(tcx, item.id)),
2232+
ast::MethodImplItem(..) | ast::MacImplItem(..) => None,
2233+
})
2234+
.flat_map(|ty| ctp::parameters_for_type(ty).into_iter())
2235+
.filter_map(|p| match p {
2236+
ctp::Parameter::Type(_) => None,
2237+
ctp::Parameter::Region(r) => Some(r),
2238+
})
2239+
.collect();
2240+
2241+
for (index, lifetime_def) in ast_generics.lifetimes.iter().enumerate() {
2242+
let region = ty::EarlyBoundRegion { param_id: lifetime_def.lifetime.id,
2243+
space: TypeSpace,
2244+
index: index as u32,
2245+
name: lifetime_def.lifetime.name };
2246+
if
2247+
lifetimes_in_associated_types.contains(&region) && // (*)
2248+
!input_parameters.contains(&ctp::Parameter::Region(region))
2249+
{
2250+
report_unused_parameter(tcx, lifetime_def.lifetime.span,
2251+
"lifetime", &region.name.user_string(tcx));
22242252
}
22252253
}
2254+
2255+
// (*) This is a horrible concession to reality. I think it'd be
2256+
// better to just ban unconstrianed lifetimes outright, but in
2257+
// practice people do non-hygenic macros like:
2258+
//
2259+
// ```
2260+
// macro_rules! __impl_slice_eq1 {
2261+
// ($Lhs: ty, $Rhs: ty, $Bound: ident) => {
2262+
// impl<'a, 'b, A: $Bound, B> PartialEq<$Rhs> for $Lhs where A: PartialEq<B> {
2263+
// ....
2264+
// }
2265+
// }
2266+
// }
2267+
// ```
2268+
//
2269+
// In a concession to backwards compatbility, we continue to
2270+
// permit those, so long as the lifetimes aren't used in
2271+
// associated types. I believe this is sound, because lifetimes
2272+
// used elsewhere are not projected back out.
2273+
}
2274+
2275+
fn report_unused_parameter(tcx: &ty::ctxt,
2276+
span: Span,
2277+
kind: &str,
2278+
name: &str)
2279+
{
2280+
span_err!(tcx.sess, span, E0207,
2281+
"the {} parameter `{}` is not constrained by the \
2282+
impl trait, self type, or predicates",
2283+
kind, name);
22262284
}
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// Test that lifetime parameters must be constrained if they appear in
12+
// an associated type def'n. Issue #22077.
13+
14+
trait Fun {
15+
type Output;
16+
fn call<'x>(&'x self) -> Self::Output;
17+
}
18+
19+
struct Holder { x: String }
20+
21+
impl<'a> Fun for Holder { //~ ERROR E0207
22+
type Output = &'a str;
23+
fn call<'b>(&'b self) -> &'b str {
24+
&self.x[..]
25+
}
26+
}
27+
28+
fn main() { }

src/test/compile-fail/issue-22886.rs

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
// Copyright 2015 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// Regression test for #22886.
12+
13+
fn crash_please() {
14+
let mut iter = Newtype(Some(Box::new(0)));
15+
let saved = iter.next().unwrap();
16+
println!("{}", saved);
17+
iter.0 = None;
18+
println!("{}", saved);
19+
}
20+
21+
struct Newtype(Option<Box<usize>>);
22+
23+
impl<'a> Iterator for Newtype { //~ ERROR E0207
24+
type Item = &'a Box<usize>;
25+
26+
fn next(&mut self) -> Option<&Box<usize>> {
27+
self.0.as_ref()
28+
}
29+
}
30+
31+
fn main() { }

0 commit comments

Comments
 (0)