From a1110bc3a33e1fd4e08e7532bb9cd28a0216016e Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Mon, 29 Jun 2015 21:07:09 +0300 Subject: [PATCH 1/5] Fix off-by-one error in default-type-parameter checking Fixes #18183 --- src/librustc_typeck/collect.rs | 45 ++++++++++++++++------------ src/test/compile-fail/issue-18183.rs | 13 ++++++++ 2 files changed, 39 insertions(+), 19 deletions(-) create mode 100644 src/test/compile-fail/issue-18183.rs diff --git a/src/librustc_typeck/collect.rs b/src/librustc_typeck/collect.rs index ef9dcd56a578b..d4f04987a812f 100644 --- a/src/librustc_typeck/collect.rs +++ b/src/librustc_typeck/collect.rs @@ -1860,6 +1860,29 @@ fn ty_generics<'a,'tcx>(ccx: &CrateCtxt<'a,'tcx>, result } +fn convert_default_type_parameter<'a, 'tcx>(ccx: &CrateCtxt<'a, 'tcx>, + path: &P, + space: ParamSpace, + index: u32) + -> Ty<'tcx> +{ + let ty = ast_ty_to_ty(&ccx.icx(&()), &ExplicitRscope, &path); + + for leaf_ty in ty.walk() { + if let ty::TyParam(p) = leaf_ty.sty { + if p.space == space && p.idx >= index { + span_err!(ccx.tcx.sess, path.span, E0128, + "type parameters with a default cannot use \ + forward declared identifiers"); + + return ccx.tcx.types.err + } + } + } + + ty +} + fn get_or_create_type_parameter_def<'a,'tcx>(ccx: &CrateCtxt<'a,'tcx>, ast_generics: &ast::Generics, space: ParamSpace, @@ -1874,25 +1897,9 @@ fn get_or_create_type_parameter_def<'a,'tcx>(ccx: &CrateCtxt<'a,'tcx>, None => { } } - let default = match param.default { - None => None, - Some(ref path) => { - let ty = ast_ty_to_ty(&ccx.icx(&()), &ExplicitRscope, &**path); - let cur_idx = index; - - for leaf_ty in ty.walk() { - if let ty::TyParam(p) = leaf_ty.sty { - if p.idx > cur_idx { - span_err!(tcx.sess, path.span, E0128, - "type parameters with a default cannot use \ - forward declared identifiers"); - } - } - } - - Some(ty) - } - }; + let default = param.default.as_ref().map( + |def| convert_default_type_parameter(ccx, def, space, index) + ); let object_lifetime_default = compute_object_lifetime_default(ccx, param.id, diff --git a/src/test/compile-fail/issue-18183.rs b/src/test/compile-fail/issue-18183.rs new file mode 100644 index 0000000000000..e6f3a2bdd33ad --- /dev/null +++ b/src/test/compile-fail/issue-18183.rs @@ -0,0 +1,13 @@ +// 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. + +pub struct Foo; //~ ERROR E0128 +pub struct Baz(Foo); +fn main() {} From bf164bc6e3b81b9999ad4baddb91079a99f49ab4 Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Tue, 30 Jun 2015 00:32:39 +0300 Subject: [PATCH 2/5] Fix lifetime elision region accounting This merges accumulate_regions_in_type with ty_fold::collect_regions. Fixes #26638 --- .../middle/infer/higher_ranked/mod.rs | 15 +- src/librustc/middle/ty.rs | 67 ++---- src/librustc/middle/ty_fold.rs | 30 ++- src/librustc/middle/ty_walk.rs | 1 + src/librustc_typeck/astconv.rs | 199 +++++++++--------- src/librustc_typeck/check/mod.rs | 4 +- src/librustc_typeck/collect.rs | 2 +- src/librustc_typeck/rscope.rs | 29 ++- src/test/compile-fail/issue-26638.rs | 19 ++ 9 files changed, 187 insertions(+), 179 deletions(-) create mode 100644 src/test/compile-fail/issue-26638.rs diff --git a/src/librustc/middle/infer/higher_ranked/mod.rs b/src/librustc/middle/infer/higher_ranked/mod.rs index 9005e1b8c53a5..64063623f6776 100644 --- a/src/librustc/middle/infer/higher_ranked/mod.rs +++ b/src/librustc/middle/infer/higher_ranked/mod.rs @@ -359,7 +359,7 @@ fn fold_regions_in<'tcx, T, F>(tcx: &ty::ctxt<'tcx>, where T: TypeFoldable<'tcx>, F: FnMut(ty::Region, ty::DebruijnIndex) -> ty::Region, { - unbound_value.fold_with(&mut ty_fold::RegionFolder::new(tcx, &mut |region, current_depth| { + ty_fold::fold_regions(tcx, unbound_value, &mut false, |region, current_depth| { // we should only be encountering "escaping" late-bound regions here, // because the ones at the current level should have been replaced // with fresh variables @@ -369,7 +369,7 @@ fn fold_regions_in<'tcx, T, F>(tcx: &ty::ctxt<'tcx>, }); fldr(region, ty::DebruijnIndex::new(current_depth)) - })) + }) } impl<'a,'tcx> InferCtxtExt for InferCtxt<'a,'tcx> { @@ -437,11 +437,10 @@ impl<'a,'tcx> InferCtxtExt for InferCtxt<'a,'tcx> { let escaping_types = self.type_variables.borrow().types_escaping_snapshot(&snapshot.type_snapshot); - let escaping_region_vars: FnvHashSet<_> = - escaping_types - .iter() - .flat_map(|&t| ty_fold::collect_regions(self.tcx, &t)) - .collect(); + let mut escaping_region_vars = FnvHashSet(); + for ty in &escaping_types { + ty_fold::collect_regions(self.tcx, ty, &mut escaping_region_vars); + } region_vars.retain(|®ion_vid| { let r = ty::ReInfer(ty::ReVar(region_vid)); @@ -649,7 +648,7 @@ pub fn plug_leaks<'a,'tcx,T>(infcx: &InferCtxt<'a,'tcx>, // binder is that we encountered in `value`. The caller is // responsible for ensuring that (a) `value` contains at least one // binder and (b) that binder is the one we want to use. - let result = ty_fold::fold_regions(infcx.tcx, &value, |r, current_depth| { + let result = ty_fold::fold_regions(infcx.tcx, &value, &mut false, |r, current_depth| { match inv_skol_map.get(&r) { None => r, Some(br) => { diff --git a/src/librustc/middle/ty.rs b/src/librustc/middle/ty.rs index 489ce7bc4cf78..1bed9c4e0a834 100644 --- a/src/librustc/middle/ty.rs +++ b/src/librustc/middle/ty.rs @@ -1713,6 +1713,16 @@ impl Region { _ => false, } } + + /// Returns the depth of `self` from the (1-based) binding level `depth` + pub fn from_depth(&self, depth: u32) -> Region { + match *self { + ty::ReLateBound(debruijn, r) => ty::ReLateBound(DebruijnIndex { + depth: debruijn.depth - (depth - 1) + }, r), + r => r + } + } } #[derive(Clone, PartialEq, PartialOrd, Eq, Ord, Hash, @@ -6783,60 +6793,6 @@ pub enum ExplicitSelfCategory { ByBoxExplicitSelfCategory, } -impl<'tcx> TyS<'tcx> { - /// Pushes all the lifetimes in the given type onto the given list. A - /// "lifetime in a type" is a lifetime specified by a reference or a lifetime - /// in a list of type substitutions. This does *not* traverse into nominal - /// types, nor does it resolve fictitious types. - pub fn accumulate_lifetimes_in_type(&self, accumulator: &mut Vec) { - for ty in self.walk() { - match ty.sty { - TyRef(region, _) => { - accumulator.push(*region) - } - TyTrait(ref t) => { - accumulator.push_all(t.principal.0.substs.regions().as_slice()); - } - TyEnum(_, substs) | - TyStruct(_, substs) => { - accum_substs(accumulator, substs); - } - TyClosure(_, substs) => { - accum_substs(accumulator, substs); - } - TyBool | - TyChar | - TyInt(_) | - TyUint(_) | - TyFloat(_) | - TyBox(_) | - TyStr | - TyArray(_, _) | - TySlice(_) | - TyRawPtr(_) | - TyBareFn(..) | - TyTuple(_) | - TyProjection(_) | - TyParam(_) | - TyInfer(_) | - TyError => { - } - } - } - - fn accum_substs(accumulator: &mut Vec, substs: &Substs) { - match substs.regions { - subst::ErasedRegions => {} - subst::NonerasedRegions(ref regions) => { - for region in regions { - accumulator.push(*region) - } - } - } - } - } -} - /// A free variable referred to in a function. #[derive(Copy, Clone, RustcEncodable, RustcDecodable)] pub struct Freevar { @@ -6917,7 +6873,8 @@ impl<'tcx> ctxt<'tcx> { where T: TypeFoldable<'tcx> { let bound0_value = bound2_value.skip_binder().skip_binder(); - let value = ty_fold::fold_regions(self, bound0_value, |region, current_depth| { + let value = ty_fold::fold_regions(self, bound0_value, &mut false, + |region, current_depth| { match region { ty::ReLateBound(debruijn, br) if debruijn.depth >= current_depth => { // should be true if no escaping regions from bound2_value diff --git a/src/librustc/middle/ty_fold.rs b/src/librustc/middle/ty_fold.rs index 012f5216ed7e3..284d26b3cd6d7 100644 --- a/src/librustc/middle/ty_fold.rs +++ b/src/librustc/middle/ty_fold.rs @@ -44,7 +44,7 @@ use std::rc::Rc; use syntax::abi; use syntax::ast; use syntax::owned_slice::OwnedSlice; -use util::nodemap::FnvHashMap; +use util::nodemap::{FnvHashMap, FnvHashSet}; /////////////////////////////////////////////////////////////////////////// // Two generic traits @@ -783,38 +783,51 @@ impl<'a, 'tcx, F> TypeFolder<'tcx> for BottomUpFolder<'a, 'tcx, F> where pub struct RegionFolder<'a, 'tcx: 'a> { tcx: &'a ty::ctxt<'tcx>, + skipped_regions: &'a mut bool, current_depth: u32, fld_r: &'a mut (FnMut(ty::Region, u32) -> ty::Region + 'a), } impl<'a, 'tcx> RegionFolder<'a, 'tcx> { - pub fn new(tcx: &'a ty::ctxt<'tcx>, fld_r: &'a mut F) -> RegionFolder<'a, 'tcx> + pub fn new(tcx: &'a ty::ctxt<'tcx>, + skipped_regions: &'a mut bool, + fld_r: &'a mut F) -> RegionFolder<'a, 'tcx> where F : FnMut(ty::Region, u32) -> ty::Region { RegionFolder { tcx: tcx, + skipped_regions: skipped_regions, current_depth: 1, fld_r: fld_r, } } } -pub fn collect_regions<'tcx,T>(tcx: &ty::ctxt<'tcx>, value: &T) -> Vec +/// Collects the free and escaping regions in `value` into `region_set`. Returns +/// whether any late-bound regions were skipped +pub fn collect_regions<'tcx,T>(tcx: &ty::ctxt<'tcx>, + value: &T, + region_set: &mut FnvHashSet) -> bool where T : TypeFoldable<'tcx> { - let mut vec = Vec::new(); - fold_regions(tcx, value, |r, _| { vec.push(r); r }); - vec + let mut have_bound_regions = false; + fold_regions(tcx, value, &mut have_bound_regions, + |r, d| { region_set.insert(r.from_depth(d)); r }); + have_bound_regions } +/// Folds the escaping and free regions in `value` using `f`, and +/// sets `skipped_regions` to true if any late-bound region was found +/// and skipped. pub fn fold_regions<'tcx,T,F>(tcx: &ty::ctxt<'tcx>, value: &T, + skipped_regions: &mut bool, mut f: F) -> T where F : FnMut(ty::Region, u32) -> ty::Region, T : TypeFoldable<'tcx>, { - value.fold_with(&mut RegionFolder::new(tcx, &mut f)) + value.fold_with(&mut RegionFolder::new(tcx, skipped_regions, &mut f)) } impl<'a, 'tcx> TypeFolder<'tcx> for RegionFolder<'a, 'tcx> @@ -834,6 +847,7 @@ impl<'a, 'tcx> TypeFolder<'tcx> for RegionFolder<'a, 'tcx> ty::ReLateBound(debruijn, _) if debruijn.depth < self.current_depth => { debug!("RegionFolder.fold_region({:?}) skipped bound region (current depth={})", r, self.current_depth); + *self.skipped_regions = true; r } _ => { @@ -989,7 +1003,7 @@ pub fn shift_regions<'tcx, T:TypeFoldable<'tcx>>(tcx: &ty::ctxt<'tcx>, debug!("shift_regions(value={:?}, amount={})", value, amount); - value.fold_with(&mut RegionFolder::new(tcx, &mut |region, _current_depth| { + value.fold_with(&mut RegionFolder::new(tcx, &mut false, &mut |region, _current_depth| { shift_region(region, amount) })) } diff --git a/src/librustc/middle/ty_walk.rs b/src/librustc/middle/ty_walk.rs index b34eb2ddb1e91..3e9a402f9499c 100644 --- a/src/librustc/middle/ty_walk.rs +++ b/src/librustc/middle/ty_walk.rs @@ -9,6 +9,7 @@ // except according to those terms. //! An iterator over the type substructure. +//! WARNING: this does not keep track of the region depth. use middle::ty::{self, Ty}; use std::iter::Iterator; diff --git a/src/librustc_typeck/astconv.rs b/src/librustc_typeck/astconv.rs index 2575dc0184f8c..a55a8ff109e66 100644 --- a/src/librustc_typeck/astconv.rs +++ b/src/librustc_typeck/astconv.rs @@ -59,7 +59,8 @@ use middle::traits; use middle::ty::{self, RegionEscape, Ty, ToPredicate, HasTypeFlags}; use middle::ty_fold; use rscope::{self, UnelidableRscope, RegionScope, ElidableRscope, ExplicitRscope, - ObjectLifetimeDefaultRscope, ShiftedRscope, BindingRscope}; + ObjectLifetimeDefaultRscope, ShiftedRscope, BindingRscope, + ElisionFailureInfo, ElidedLifetime}; use util::common::{ErrorReported, FN_OUTPUT_NAME}; use util::nodemap::FnvHashSet; @@ -186,6 +187,58 @@ pub fn ast_region_to_region(tcx: &ty::ctxt, lifetime: &ast::Lifetime) r } +fn report_elision_failure( + tcx: &ty::ctxt, + default_span: Span, + params: Vec) +{ + let mut m = String::new(); + let len = params.len(); + for (i, info) in params.into_iter().enumerate() { + let ElisionFailureInfo { + name, lifetime_count: n, have_bound_regions + } = info; + + let help_name = if name.is_empty() { + format!("argument {}", i + 1) + } else { + format!("`{}`", name) + }; + + m.push_str(&(if n == 1 { + help_name + } else { + format!("one of {}'s {} elided {}lifetimes", help_name, n, + if have_bound_regions { "free " } else { "" } ) + })[..]); + + if len == 2 && i == 0 { + m.push_str(" or "); + } else if i + 2 == len { + m.push_str(", or "); + } else if i + 1 != len { + m.push_str(", "); + } + } + if len == 1 { + fileline_help!(tcx.sess, default_span, + "this function's return type contains a borrowed value, but \ + the signature does not say which {} it is borrowed from", + m); + } else if len == 0 { + fileline_help!(tcx.sess, default_span, + "this function's return type contains a borrowed value, but \ + there is no value for it to be borrowed from"); + fileline_help!(tcx.sess, default_span, + "consider giving it a 'static lifetime"); + } else { + fileline_help!(tcx.sess, default_span, + "this function's return type contains a borrowed value, but \ + the signature does not say whether it is borrowed from {}", + m); + } +} + pub fn opt_ast_region_to_region<'tcx>( this: &AstConv<'tcx>, rscope: &RegionScope, @@ -197,61 +250,15 @@ pub fn opt_ast_region_to_region<'tcx>( ast_region_to_region(this.tcx(), lifetime) } - None => { - match rscope.anon_regions(default_span, 1) { - Err(v) => { - debug!("optional region in illegal location"); - span_err!(this.tcx().sess, default_span, E0106, - "missing lifetime specifier"); - match v { - Some(v) => { - let mut m = String::new(); - let len = v.len(); - for (i, (name, n)) in v.into_iter().enumerate() { - let help_name = if name.is_empty() { - format!("argument {}", i + 1) - } else { - format!("`{}`", name) - }; - - m.push_str(&(if n == 1 { - help_name - } else { - format!("one of {}'s {} elided lifetimes", help_name, n) - })[..]); - - if len == 2 && i == 0 { - m.push_str(" or "); - } else if i + 2 == len { - m.push_str(", or "); - } else if i + 1 != len { - m.push_str(", "); - } - } - if len == 1 { - fileline_help!(this.tcx().sess, default_span, - "this function's return type contains a borrowed value, but \ - the signature does not say which {} it is borrowed from", - m); - } else if len == 0 { - fileline_help!(this.tcx().sess, default_span, - "this function's return type contains a borrowed value, but \ - there is no value for it to be borrowed from"); - fileline_help!(this.tcx().sess, default_span, - "consider giving it a 'static lifetime"); - } else { - fileline_help!(this.tcx().sess, default_span, - "this function's return type contains a borrowed value, but \ - the signature does not say whether it is borrowed from {}", - m); - } - } - None => {}, - } - ty::ReStatic + None => match rscope.anon_regions(default_span, 1) { + Ok(rs) => rs[0], + Err(params) => { + span_err!(this.tcx().sess, default_span, E0106, + "missing lifetime specifier"); + if let Some(params) = params { + report_elision_failure(this.tcx(), default_span, params); } - - Ok(rs) => rs[0], + ty::ReStatic } } }; @@ -505,48 +512,54 @@ fn convert_angle_bracketed_parameters<'tcx>(this: &AstConv<'tcx>, /// Returns the appropriate lifetime to use for any output lifetimes /// (if one exists) and a vector of the (pattern, number of lifetimes) /// corresponding to each input type/pattern. -fn find_implied_output_region(input_tys: &[Ty], input_pats: Vec) - -> (Option, Vec<(String, usize)>) +fn find_implied_output_region<'tcx>(tcx: &ty::ctxt<'tcx>, + input_tys: &[Ty<'tcx>], + input_pats: Vec) -> ElidedLifetime { - let mut lifetimes_for_params: Vec<(String, usize)> = Vec::new(); + let mut lifetimes_for_params = Vec::new(); let mut possible_implied_output_region = None; for (input_type, input_pat) in input_tys.iter().zip(input_pats) { - let mut accumulator = Vec::new(); - input_type.accumulate_lifetimes_in_type(&mut accumulator); + let mut regions = FnvHashSet(); + let have_bound_regions = ty_fold::collect_regions(tcx, + input_type, + &mut regions); - if accumulator.len() == 1 { + debug!("find_implied_output_regions: collected {:?} from {:?} \ + have_bound_regions={:?}", ®ions, input_type, have_bound_regions); + + if regions.len() == 1 { // there's a chance that the unique lifetime of this // iteration will be the appropriate lifetime for output // parameters, so lets store it. - possible_implied_output_region = Some(accumulator[0]) + possible_implied_output_region = regions.iter().cloned().next(); } - lifetimes_for_params.push((input_pat, accumulator.len())); + lifetimes_for_params.push(ElisionFailureInfo { + name: input_pat, + lifetime_count: regions.len(), + have_bound_regions: have_bound_regions + }); } - let implied_output_region = - if lifetimes_for_params.iter().map(|&(_, n)| n).sum::() == 1 { - assert!(possible_implied_output_region.is_some()); - possible_implied_output_region - } else { - None - }; - (implied_output_region, lifetimes_for_params) + if lifetimes_for_params.iter().map(|e| e.lifetime_count).sum::() == 1 { + Ok(possible_implied_output_region.unwrap()) + } else { + Err(Some(lifetimes_for_params)) + } } fn convert_ty_with_lifetime_elision<'tcx>(this: &AstConv<'tcx>, - implied_output_region: Option, - param_lifetimes: Vec<(String, usize)>, + elided_lifetime: ElidedLifetime, ty: &ast::Ty) -> Ty<'tcx> { - match implied_output_region { - Some(implied_output_region) => { + match elided_lifetime { + Ok(implied_output_region) => { let rb = ElidableRscope::new(implied_output_region); ast_ty_to_ty(this, &rb, ty) } - None => { + Err(param_lifetimes) => { // All regions must be explicitly specified in the output // if the lifetime elision rules do not apply. This saves // the user from potentially-confusing errors. @@ -576,8 +589,7 @@ fn convert_parenthesized_parameters<'tcx>(this: &AstConv<'tcx>, .collect::>>(); let input_params: Vec<_> = repeat(String::new()).take(inputs.len()).collect(); - let (implied_output_region, - params_lifetimes) = find_implied_output_region(&*inputs, input_params); + let implied_output_region = find_implied_output_region(this.tcx(), &inputs, input_params); let input_ty = this.tcx().mk_tup(inputs); @@ -585,8 +597,7 @@ fn convert_parenthesized_parameters<'tcx>(this: &AstConv<'tcx>, Some(ref output_ty) => { (convert_ty_with_lifetime_elision(this, implied_output_region, - params_lifetimes, - &**output_ty), + &output_ty), output_ty.span) } None => { @@ -1705,7 +1716,7 @@ fn ty_of_method_or_bare_fn<'a, 'tcx>(this: &AstConv<'tcx>, // here), if self is by-reference, then the implied output region is the // region of the self parameter. let mut explicit_self_category_result = None; - let (self_ty, mut implied_output_region) = match opt_self_info { + let (self_ty, implied_output_region) = match opt_self_info { None => (None, None), Some(self_info) => { // This type comes from an impl or trait; no late-bound @@ -1756,19 +1767,18 @@ fn ty_of_method_or_bare_fn<'a, 'tcx>(this: &AstConv<'tcx>, // Second, if there was exactly one lifetime (either a substitution or a // reference) in the arguments, then any anonymous regions in the output // have that lifetime. - let lifetimes_for_params = if implied_output_region.is_none() { - let input_tys = if self_ty.is_some() { - // Skip the first argument if `self` is present. - &self_and_input_tys[1..] - } else { - &self_and_input_tys[..] - }; + let implied_output_region = match implied_output_region { + Some(r) => Ok(r), + None => { + let input_tys = if self_ty.is_some() { + // Skip the first argument if `self` is present. + &self_and_input_tys[1..] + } else { + &self_and_input_tys[..] + }; - let (ior, lfp) = find_implied_output_region(input_tys, input_pats); - implied_output_region = ior; - lfp - } else { - vec![] + find_implied_output_region(this.tcx(), input_tys, input_pats) + } }; let output_ty = match decl.output { @@ -1777,8 +1787,7 @@ fn ty_of_method_or_bare_fn<'a, 'tcx>(this: &AstConv<'tcx>, ast::Return(ref output) => ty::FnConverging(convert_ty_with_lifetime_elision(this, implied_output_region, - lifetimes_for_params, - &**output)), + &output)), ast::DefaultReturn(..) => ty::FnConverging(this.tcx().mk_nil()), ast::NoReturn(..) => ty::FnDiverging }; diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 7e87dc6540ea5..2db1598db4bee 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -97,7 +97,7 @@ use middle::ty::{Disr, ParamTy, ParameterEnvironment}; use middle::ty::{self, HasTypeFlags, RegionEscape, ToPolyTraitRef, Ty}; use middle::ty::{MethodCall, MethodCallee}; use middle::ty_fold::{TypeFolder, TypeFoldable}; -use rscope::RegionScope; +use rscope::{ElisionFailureInfo, RegionScope}; use session::Session; use {CrateCtxt, lookup_full_def, require_same_types}; use TypeAndSubsts; @@ -1796,7 +1796,7 @@ impl<'a, 'tcx> RegionScope for FnCtxt<'a, 'tcx> { } fn anon_regions(&self, span: Span, count: usize) - -> Result, Option>> { + -> Result, Option>> { Ok((0..count).map(|_| { self.infcx().next_region_var(infer::MiscVariable(span)) }).collect()) diff --git a/src/librustc_typeck/collect.rs b/src/librustc_typeck/collect.rs index d4f04987a812f..7909908079fb3 100644 --- a/src/librustc_typeck/collect.rs +++ b/src/librustc_typeck/collect.rs @@ -2248,7 +2248,7 @@ fn check_method_self_type<'a, 'tcx, RS:RegionScope>( * before we really have a `ParameterEnvironment` to check. */ - ty_fold::fold_regions(tcx, value, |region, _| { + ty_fold::fold_regions(tcx, value, &mut false, |region, _| { match region { ty::ReEarlyBound(data) => { let def_id = local_def(data.param_id); diff --git a/src/librustc_typeck/rscope.rs b/src/librustc_typeck/rscope.rs index c908e21626e56..b416026ba0134 100644 --- a/src/librustc_typeck/rscope.rs +++ b/src/librustc_typeck/rscope.rs @@ -16,6 +16,15 @@ use std::cell::Cell; use std::iter::repeat; use syntax::codemap::Span; +#[derive(Clone)] +pub struct ElisionFailureInfo { + pub name: String, + pub lifetime_count: usize, + pub have_bound_regions: bool +} + +pub type ElidedLifetime = Result>>; + /// Defines strategies for handling regions that are omitted. For /// example, if one writes the type `&Foo`, then the lifetime of /// this reference has been omitted. When converting this @@ -30,7 +39,7 @@ pub trait RegionScope { fn anon_regions(&self, span: Span, count: usize) - -> Result, Option>>; + -> Result, Option>>; /// If an object omits any explicit lifetime bound, and none can /// be derived from the object traits, what should we use? If @@ -51,16 +60,16 @@ impl RegionScope for ExplicitRscope { fn anon_regions(&self, _span: Span, _count: usize) - -> Result, Option>> { + -> Result, Option>> { Err(None) } } // Same as `ExplicitRscope`, but provides some extra information for diagnostics -pub struct UnelidableRscope(Vec<(String, usize)>); +pub struct UnelidableRscope(Option>); impl UnelidableRscope { - pub fn new(v: Vec<(String, usize)>) -> UnelidableRscope { + pub fn new(v: Option>) -> UnelidableRscope { UnelidableRscope(v) } } @@ -73,9 +82,9 @@ impl RegionScope for UnelidableRscope { fn anon_regions(&self, _span: Span, _count: usize) - -> Result, Option>> { + -> Result, Option>> { let UnelidableRscope(ref v) = *self; - Err(Some(v.clone())) + Err(v.clone()) } } @@ -104,7 +113,7 @@ impl RegionScope for ElidableRscope { fn anon_regions(&self, _span: Span, count: usize) - -> Result, Option>> + -> Result, Option>> { Ok(repeat(self.default).take(count).collect()) } @@ -141,7 +150,7 @@ impl RegionScope for BindingRscope { fn anon_regions(&self, _: Span, count: usize) - -> Result, Option>> + -> Result, Option>> { Ok((0..count).map(|_| self.next_region()).collect()) } @@ -177,7 +186,7 @@ impl<'r> RegionScope for ObjectLifetimeDefaultRscope<'r> { fn anon_regions(&self, span: Span, count: usize) - -> Result, Option>> + -> Result, Option>> { self.base_scope.anon_regions(span, count) } @@ -204,7 +213,7 @@ impl<'r> RegionScope for ShiftedRscope<'r> { fn anon_regions(&self, span: Span, count: usize) - -> Result, Option>> + -> Result, Option>> { match self.base_scope.anon_regions(span, count) { Ok(mut v) => { diff --git a/src/test/compile-fail/issue-26638.rs b/src/test/compile-fail/issue-26638.rs new file mode 100644 index 0000000000000..edb9ab47fc611 --- /dev/null +++ b/src/test/compile-fail/issue-26638.rs @@ -0,0 +1,19 @@ +// 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. + +fn parse_type(iter: Box+'static>) -> &str { iter.next() } +//~^ ERROR missing lifetime specifier [E0106] +//~^^ HELP 2 elided lifetimes + +fn parse_type_2(iter: fn(&u8)->&u8) -> &str { iter() } +//~^ ERROR missing lifetime specifier [E0106] +//~^^ HELP 0 elided free lifetimes + +fn main() {} From 336f81215eb166aad4f8759be0cf284f3321212f Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Tue, 30 Jun 2015 01:34:17 +0300 Subject: [PATCH 3/5] Remove type_needs_unwind_cleanup After the last @dinosaur went extinct, the check became redundant with type_needs_drop, except for its bugginess. Fixes #26655. --- src/librustc_trans/trans/cleanup.rs | 9 ++---- src/librustc_trans/trans/common.rs | 45 +---------------------------- src/test/run-pass/issue-26655.rs | 34 ++++++++++++++++++++++ 3 files changed, 37 insertions(+), 51 deletions(-) create mode 100644 src/test/run-pass/issue-26655.rs diff --git a/src/librustc_trans/trans/cleanup.rs b/src/librustc_trans/trans/cleanup.rs index b7e761fa4b991..0b4d7e602be80 100644 --- a/src/librustc_trans/trans/cleanup.rs +++ b/src/librustc_trans/trans/cleanup.rs @@ -389,7 +389,6 @@ impl<'blk, 'tcx> CleanupMethods<'blk, 'tcx> for FunctionContext<'blk, 'tcx> { if !self.type_needs_drop(ty) { return; } let drop = box DropValue { is_immediate: false, - must_unwind: common::type_needs_unwind_cleanup(self.ccx, ty), val: val, ty: ty, fill_on_drop: false, @@ -415,7 +414,6 @@ impl<'blk, 'tcx> CleanupMethods<'blk, 'tcx> for FunctionContext<'blk, 'tcx> { let drop = box DropValue { is_immediate: false, - must_unwind: common::type_needs_unwind_cleanup(self.ccx, ty), val: val, ty: ty, fill_on_drop: true, @@ -447,7 +445,6 @@ impl<'blk, 'tcx> CleanupMethods<'blk, 'tcx> for FunctionContext<'blk, 'tcx> { let drop = box DropValue { is_immediate: false, - must_unwind: common::type_needs_unwind_cleanup(self.ccx, ty), val: val, ty: ty, fill_on_drop: false, @@ -473,7 +470,6 @@ impl<'blk, 'tcx> CleanupMethods<'blk, 'tcx> for FunctionContext<'blk, 'tcx> { if !self.type_needs_drop(ty) { return; } let drop = box DropValue { is_immediate: true, - must_unwind: common::type_needs_unwind_cleanup(self.ccx, ty), val: val, ty: ty, fill_on_drop: false, @@ -1031,7 +1027,6 @@ impl EarlyExitLabel { #[derive(Copy, Clone)] pub struct DropValue<'tcx> { is_immediate: bool, - must_unwind: bool, val: ValueRef, ty: Ty<'tcx>, fill_on_drop: bool, @@ -1040,11 +1035,11 @@ pub struct DropValue<'tcx> { impl<'tcx> Cleanup<'tcx> for DropValue<'tcx> { fn must_unwind(&self) -> bool { - self.must_unwind + true } fn clean_on_unwind(&self) -> bool { - self.must_unwind + true } fn is_lifetime_end(&self) -> bool { diff --git a/src/librustc_trans/trans/common.rs b/src/librustc_trans/trans/common.rs index 483d82f508f25..9c2aea1e67adf 100644 --- a/src/librustc_trans/trans/common.rs +++ b/src/librustc_trans/trans/common.rs @@ -25,7 +25,7 @@ use middle::lang_items::LangItem; use middle::mem_categorization as mc; use middle::mem_categorization::Typer; use middle::region; -use middle::subst::{self, Subst, Substs}; +use middle::subst::{self, Substs}; use trans::base; use trans::build; use trans::cleanup; @@ -54,8 +54,6 @@ use syntax::ast; use syntax::codemap::{DUMMY_SP, Span}; use syntax::parse::token::InternedString; use syntax::parse::token; -use util::common::memoized; -use util::nodemap::FnvHashSet; pub use trans::context::CrateContext; @@ -136,47 +134,6 @@ pub fn type_is_fat_ptr<'tcx>(cx: &ty::ctxt<'tcx>, ty: Ty<'tcx>) -> bool { } } -// Some things don't need cleanups during unwinding because the -// thread can free them all at once later. Currently only things -// that only contain scalars and shared boxes can avoid unwind -// cleanups. -pub fn type_needs_unwind_cleanup<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, ty: Ty<'tcx>) -> bool { - return memoized(ccx.needs_unwind_cleanup_cache(), ty, |ty| { - type_needs_unwind_cleanup_(ccx.tcx(), ty, &mut FnvHashSet()) - }); - - fn type_needs_unwind_cleanup_<'tcx>(tcx: &ty::ctxt<'tcx>, - ty: Ty<'tcx>, - tycache: &mut FnvHashSet>) - -> bool - { - // Prevent infinite recursion - if !tycache.insert(ty) { - return false; - } - - let mut needs_unwind_cleanup = false; - ty.maybe_walk(|ty| { - needs_unwind_cleanup |= match ty.sty { - ty::TyBool | ty::TyInt(_) | ty::TyUint(_) | - ty::TyFloat(_) | ty::TyTuple(_) | ty::TyRawPtr(_) => false, - - ty::TyEnum(did, substs) => - tcx.enum_variants(did).iter().any(|v| - v.args.iter().any(|&aty| { - let t = aty.subst(tcx, substs); - type_needs_unwind_cleanup_(tcx, t, tycache) - }) - ), - - _ => true - }; - !needs_unwind_cleanup - }); - needs_unwind_cleanup - } -} - /// If `type_needs_drop` returns true, then `ty` is definitely /// non-copy and *might* have a destructor attached; if it returns /// false, then `ty` definitely has no destructor (i.e. no drop glue). diff --git a/src/test/run-pass/issue-26655.rs b/src/test/run-pass/issue-26655.rs new file mode 100644 index 0000000000000..bdd3a80d74cf4 --- /dev/null +++ b/src/test/run-pass/issue-26655.rs @@ -0,0 +1,34 @@ +// 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. + +#![feature(const_fn)] + +// Check that the destructors of simple enums are run on unwinding + +use std::sync::atomic::{Ordering, AtomicUsize}; +use std::thread; + +static LOG: AtomicUsize = AtomicUsize::new(0); + +enum WithDtor { Val } +impl Drop for WithDtor { + fn drop(&mut self) { + LOG.store(LOG.load(Ordering::SeqCst)+1,Ordering::SeqCst); + } +} + +pub fn main() { + thread::spawn(move|| { + let _e: WithDtor = WithDtor::Val; + panic!("fail"); + }).join().unwrap_err(); + + assert_eq!(LOG.load(Ordering::SeqCst), 1); +} From a5e21daa1909f538ddd696f7baffad4603f38a5d Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Tue, 30 Jun 2015 02:24:46 +0300 Subject: [PATCH 4/5] Kill count_late_bound_regions No, it didn't show up in my profiler. It still needs to die. --- src/librustc/middle/traits/fulfill.rs | 24 ++++++++--------- src/librustc/middle/ty.rs | 39 ++++++++++++++++----------- src/librustc_typeck/astconv.rs | 8 +++--- 3 files changed, 40 insertions(+), 31 deletions(-) diff --git a/src/librustc/middle/traits/fulfill.rs b/src/librustc/middle/traits/fulfill.rs index 5e274dcec70e4..dc3ccd417b8f7 100644 --- a/src/librustc/middle/traits/fulfill.rs +++ b/src/librustc/middle/traits/fulfill.rs @@ -421,16 +421,18 @@ fn process_predicate<'a,'tcx>(selcx: &mut SelectionContext<'a,'tcx>, // regions. If there are, we will call this obligation an // error. Eventually we should be able to support some // cases here, I imagine (e.g., `for<'a> int : 'a`). - if selcx.tcx().count_late_bound_regions(binder) != 0 { - errors.push( - FulfillmentError::new( - obligation.clone(), - CodeSelectionError(Unimplemented))); - } else { - let ty::OutlivesPredicate(t_a, r_b) = binder.0; - register_region_obligation(t_a, r_b, - obligation.cause.clone(), - region_obligations); + match selcx.tcx().no_late_bound_regions(binder) { + None => { + errors.push( + FulfillmentError::new( + obligation.clone(), + CodeSelectionError(Unimplemented))) + } + Some(ty::OutlivesPredicate(t_a, r_b)) => { + register_region_obligation(t_a, r_b, + obligation.cause.clone(), + region_obligations); + } } true } @@ -501,5 +503,3 @@ impl<'tcx> FulfilledPredicates<'tcx> { !self.set.insert(p.clone()) } } - - diff --git a/src/librustc/middle/ty.rs b/src/librustc/middle/ty.rs index 1bed9c4e0a834..fe52fba49c6e5 100644 --- a/src/librustc/middle/ty.rs +++ b/src/librustc/middle/ty.rs @@ -6853,19 +6853,6 @@ impl<'tcx> ctxt<'tcx> { |br| ty::ReFree(ty::FreeRegion{scope: all_outlive_scope, bound_region: br})).0 } - pub fn count_late_bound_regions(&self, value: &Binder) -> usize - where T : TypeFoldable<'tcx> - { - let (_, skol_map) = ty_fold::replace_late_bound_regions(self, value, |_| ty::ReStatic); - skol_map.len() - } - - pub fn binds_late_bound_regions(&self, value: &Binder) -> bool - where T : TypeFoldable<'tcx> - { - self.count_late_bound_regions(value) > 0 - } - /// Flattens two binding levels into one. So `for<'a> for<'b> Foo` /// becomes `for<'a,'b> Foo`. pub fn flatten_late_bound_regions(&self, bound2_value: &Binder>) @@ -6890,9 +6877,9 @@ impl<'tcx> ctxt<'tcx> { } pub fn no_late_bound_regions(&self, value: &Binder) -> Option - where T : TypeFoldable<'tcx> + where T : TypeFoldable<'tcx> + RegionEscape { - if self.binds_late_bound_regions(value) { + if value.0.has_escaping_regions() { None } else { Some(value.0.clone()) @@ -7052,6 +7039,19 @@ impl<'tcx> RegionEscape for Substs<'tcx> { } } +impl RegionEscape for Vec { + fn has_regions_escaping_depth(&self, depth: u32) -> bool { + self.iter().any(|t| t.has_regions_escaping_depth(depth)) + } +} + +impl<'tcx> RegionEscape for FnSig<'tcx> { + fn has_regions_escaping_depth(&self, depth: u32) -> bool { + self.inputs.has_regions_escaping_depth(depth) || + self.output.has_regions_escaping_depth(depth) + } +} + impl<'tcx,T:RegionEscape> RegionEscape for VecPerParamSpace { fn has_regions_escaping_depth(&self, depth: u32) -> bool { self.iter_enumerated().any(|(space, _, t)| { @@ -7124,6 +7124,15 @@ impl<'tcx,T:RegionEscape> RegionEscape for Binder { } } +impl<'tcx> RegionEscape for FnOutput<'tcx> { + fn has_regions_escaping_depth(&self, depth: u32) -> bool { + match *self { + FnConverging(t) => t.has_regions_escaping_depth(depth), + FnDiverging => false + } + } +} + impl<'tcx> RegionEscape for EquatePredicate<'tcx> { fn has_regions_escaping_depth(&self, depth: u32) -> bool { self.0.has_regions_escaping_depth(depth) || self.1.has_regions_escaping_depth(depth) diff --git a/src/librustc_typeck/astconv.rs b/src/librustc_typeck/astconv.rs index a55a8ff109e66..00b7f42061405 100644 --- a/src/librustc_typeck/astconv.rs +++ b/src/librustc_typeck/astconv.rs @@ -125,14 +125,14 @@ pub trait AstConv<'tcx> { item_name: ast::Name) -> Ty<'tcx> { - if self.tcx().binds_late_bound_regions(&poly_trait_ref) { + if let Some(trait_ref) = self.tcx().no_late_bound_regions(&poly_trait_ref) { + self.projected_ty(span, trait_ref, item_name) + } else { + // no late-bound regions, we can just ignore the binder span_err!(self.tcx().sess, span, E0212, "cannot extract an associated type from a higher-ranked trait bound \ in this context"); self.tcx().types.err - } else { - // no late-bound regions, we can just ignore the binder - self.projected_ty(span, poly_trait_ref.0.clone(), item_name) } } From fb5dd398f677213e8f0943638c9205172a634efe Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Tue, 30 Jun 2015 02:31:07 +0300 Subject: [PATCH 5/5] Remove now-useless code --- src/librustc_trans/trans/cleanup.rs | 52 +++++++---------------------- 1 file changed, 12 insertions(+), 40 deletions(-) diff --git a/src/librustc_trans/trans/cleanup.rs b/src/librustc_trans/trans/cleanup.rs index 0b4d7e602be80..588e4cea5048b 100644 --- a/src/librustc_trans/trans/cleanup.rs +++ b/src/librustc_trans/trans/cleanup.rs @@ -199,7 +199,6 @@ pub struct CachedEarlyExit { pub trait Cleanup<'tcx> { fn must_unwind(&self) -> bool; - fn clean_on_unwind(&self) -> bool; fn is_lifetime_end(&self) -> bool; fn trans<'blk>(&self, bcx: Block<'blk, 'tcx>, @@ -776,29 +775,19 @@ impl<'blk, 'tcx> CleanupHelperMethods<'blk, 'tcx> for FunctionContext<'blk, 'tcx // // At this point, `popped_scopes` is empty, and so the final block // that we return to the user is `Cleanup(AST 24)`. - while !popped_scopes.is_empty() { - let mut scope = popped_scopes.pop().unwrap(); - - if scope.cleanups.iter().any(|c| cleanup_is_suitable_for(&**c, label)) - { - let name = scope.block_name("clean"); - debug!("generating cleanups for {}", name); - let bcx_in = self.new_block(label.is_unwind(), - &name[..], - None); - let mut bcx_out = bcx_in; - for cleanup in scope.cleanups.iter().rev() { - if cleanup_is_suitable_for(&**cleanup, label) { - bcx_out = cleanup.trans(bcx_out, - scope.debug_loc); - } - } - build::Br(bcx_out, prev_llbb, DebugLoc::None); - prev_llbb = bcx_in.llbb; - } else { - debug!("no suitable cleanups in {}", - scope.block_name("clean")); + while let Some(mut scope) = popped_scopes.pop() { + let name = scope.block_name("clean"); + debug!("generating cleanups for {}", name); + let bcx_in = self.new_block(label.is_unwind(), + &name[..], + None); + let mut bcx_out = bcx_in; + for cleanup in scope.cleanups.iter().rev() { + bcx_out = cleanup.trans(bcx_out, + scope.debug_loc); } + build::Br(bcx_out, prev_llbb, DebugLoc::None); + prev_llbb = bcx_in.llbb; scope.add_cached_early_exit(label, prev_llbb); self.push_scope(scope); @@ -1038,10 +1027,6 @@ impl<'tcx> Cleanup<'tcx> for DropValue<'tcx> { true } - fn clean_on_unwind(&self) -> bool { - true - } - fn is_lifetime_end(&self) -> bool { false } @@ -1085,10 +1070,6 @@ impl<'tcx> Cleanup<'tcx> for FreeValue<'tcx> { true } - fn clean_on_unwind(&self) -> bool { - true - } - fn is_lifetime_end(&self) -> bool { false } @@ -1118,10 +1099,6 @@ impl<'tcx> Cleanup<'tcx> for LifetimeEnd { false } - fn clean_on_unwind(&self) -> bool { - true - } - fn is_lifetime_end(&self) -> bool { true } @@ -1160,11 +1137,6 @@ pub fn var_scope(tcx: &ty::ctxt, r } -fn cleanup_is_suitable_for(c: &Cleanup, - label: EarlyExitLabel) -> bool { - !label.is_unwind() || c.clean_on_unwind() -} - /////////////////////////////////////////////////////////////////////////// // These traits just exist to put the methods into this file.