Skip to content

Commit b8282dd

Browse files
committed
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.
1 parent 83b2b14 commit b8282dd

File tree

3 files changed

+127
-10
lines changed

3 files changed

+127
-10
lines changed

Diff for: src/librustc_typeck/collect.rs

+68-10
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);
@@ -2215,10 +2217,66 @@ fn enforce_impl_ty_params_are_constrained<'tcx>(tcx: &ty::ctxt<'tcx>,
22152217
idx: index as u32,
22162218
name: ty_param.ident.name };
22172219
if !input_parameters.contains(&ctp::Parameter::Type(param_ty)) {
2218-
span_err!(tcx.sess, ty_param.span, E0207,
2219-
"the type parameter `{}` is not constrained by the \
2220-
impl trait, self type, or predicates",
2221-
param_ty.user_string(tcx));
2220+
report_unused_parameter(tcx, ty_param.span, "type", &param_ty.user_string(tcx));
2221+
}
2222+
}
2223+
2224+
// Every lifetime used in an associated type must be constrained.
2225+
2226+
let lifetimes_in_associated_types: HashSet<_> =
2227+
impl_items.iter()
2228+
.filter_map(|item| match item.node {
2229+
ast::TypeImplItem(..) => Some(ty::node_id_to_type(tcx, item.id)),
2230+
ast::MethodImplItem(..) | ast::MacImplItem(..) => None,
2231+
})
2232+
.flat_map(|ty| ctp::parameters_for_type(ty).into_iter())
2233+
.filter_map(|p| match p {
2234+
ctp::Parameter::Type(_) => None,
2235+
ctp::Parameter::Region(r) => Some(r),
2236+
})
2237+
.collect();
2238+
2239+
for (index, lifetime_def) in ast_generics.lifetimes.iter().enumerate() {
2240+
let region = ty::EarlyBoundRegion { param_id: lifetime_def.lifetime.id,
2241+
space: TypeSpace,
2242+
index: index as u32,
2243+
name: lifetime_def.lifetime.name };
2244+
if
2245+
lifetimes_in_associated_types.contains(&region) && // (*)
2246+
!input_parameters.contains(&ctp::Parameter::Region(region))
2247+
{
2248+
report_unused_parameter(tcx, lifetime_def.lifetime.span,
2249+
"lifetime", &region.name.user_string(tcx));
22222250
}
22232251
}
2252+
2253+
// (*) This is a horrible concession to reality. I think it'd be
2254+
// better to just ban unconstrianed lifetimes outright, but in
2255+
// practice people do non-hygenic macros like:
2256+
//
2257+
// ```
2258+
// macro_rules! __impl_slice_eq1 {
2259+
// ($Lhs: ty, $Rhs: ty, $Bound: ident) => {
2260+
// impl<'a, 'b, A: $Bound, B> PartialEq<$Rhs> for $Lhs where A: PartialEq<B> {
2261+
// ....
2262+
// }
2263+
// }
2264+
// }
2265+
// ```
2266+
//
2267+
// In a concession to backwards compatbility, we continue to
2268+
// permit those, so long as the lifetimes aren't used in
2269+
// associated types. I believe this is sound, because lifetimes
2270+
// used elsewhere are not projected back out.
2271+
}
2272+
2273+
fn report_unused_parameter(tcx: &ty::ctxt,
2274+
span: Span,
2275+
kind: &str,
2276+
name: &str)
2277+
{
2278+
span_err!(tcx.sess, span, E0207,
2279+
"the {} parameter `{}` is not constrained by the \
2280+
impl trait, self type, or predicates",
2281+
kind, name);
22242282
}
+28
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() { }

Diff for: src/test/compile-fail/issue-22886.rs

+31
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)