From c4285359a4b94541a887dcbef659a4fdea4da91a Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Tue, 8 Nov 2016 16:18:44 -0500 Subject: [PATCH] introduce a `fudge_regions_if_ok` to address false region edges Fixes #37655. --- src/librustc/infer/fudge.rs | 137 ++++++++++++++++++++++++++ src/librustc/infer/mod.rs | 44 +-------- src/librustc/infer/type_variable.rs | 4 + src/librustc_typeck/check/mod.rs | 5 +- src/librustc_typeck/check/regionck.rs | 2 +- src/test/run-pass/issue-37655.rs | 46 +++++++++ 6 files changed, 192 insertions(+), 46 deletions(-) create mode 100644 src/librustc/infer/fudge.rs create mode 100644 src/test/run-pass/issue-37655.rs diff --git a/src/librustc/infer/fudge.rs b/src/librustc/infer/fudge.rs new file mode 100644 index 0000000000000..806b94486615f --- /dev/null +++ b/src/librustc/infer/fudge.rs @@ -0,0 +1,137 @@ +// Copyright 2012-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. + +use ty::{self, TyCtxt}; +use ty::fold::{TypeFoldable, TypeFolder}; + +use super::InferCtxt; +use super::RegionVariableOrigin; + +impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { + /// This rather funky routine is used while processing expected + /// types. What happens here is that we want to propagate a + /// coercion through the return type of a fn to its + /// argument. Consider the type of `Option::Some`, which is + /// basically `for fn(T) -> Option`. So if we have an + /// expression `Some(&[1, 2, 3])`, and that has the expected type + /// `Option<&[u32]>`, we would like to type check `&[1, 2, 3]` + /// with the expectation of `&[u32]`. This will cause us to coerce + /// from `&[u32; 3]` to `&[u32]` and make the users life more + /// pleasant. + /// + /// The way we do this is using `fudge_regions_if_ok`. What the + /// routine actually does is to start a snapshot and execute the + /// closure `f`. In our example above, what this closure will do + /// is to unify the expectation (`Option<&[u32]>`) with the actual + /// return type (`Option`, where `?T` represents the variable + /// instantiated for `T`). This will cause `?T` to be unified + /// with `&?a [u32]`, where `?a` is a fresh lifetime variable. The + /// input type (`?T`) is then returned by `f()`. + /// + /// At this point, `fudge_regions_if_ok` will normalize all type + /// variables, converting `?T` to `&?a [u32]` and end the + /// snapshot. The problem is that we can't just return this type + /// out, because it references the region variable `?a`, and that + /// region variable was popped when we popped the snapshot. + /// + /// So what we do is to keep a list (`region_vars`, in the code below) + /// of region variables created during the snapshot (here, `?a`). We + /// fold the return value and replace any such regions with a *new* + /// region variable (e.g., `?b`) and return the result (`&?b [u32]`). + /// This can then be used as the expectation for the fn argument. + /// + /// The important point here is that, for soundness purposes, the + /// regions in question are not particularly important. We will + /// use the expected types to guide coercions, but we will still + /// type-check the resulting types from those coercions against + /// the actual types (`?T`, `Option(&self, + origin: &RegionVariableOrigin, + f: F) -> Result where + F: FnOnce() -> Result, + T: TypeFoldable<'tcx>, + { + let (region_vars, value) = self.probe(|snapshot| { + let vars_at_start = self.type_variables.borrow().num_vars(); + + match f() { + Ok(value) => { + let value = self.resolve_type_vars_if_possible(&value); + + // At this point, `value` could in principle refer + // to regions that have been created during the + // snapshot (we assert below that `f()` does not + // create any new type variables, so there + // shouldn't be any of those). Once we exit + // `probe()`, those are going to be popped, so we + // will have to eliminate any references to them. + + assert_eq!(self.type_variables.borrow().num_vars(), vars_at_start, + "type variables were created during fudge_regions_if_ok"); + let region_vars = + self.region_vars.vars_created_since_snapshot( + &snapshot.region_vars_snapshot); + + Ok((region_vars, value)) + } + Err(e) => Err(e), + } + })?; + + // At this point, we need to replace any of the now-popped + // region variables that appear in `value` with a fresh region + // variable. We can't do this during the probe because they + // would just get popped then too. =) + + // Micro-optimization: if no variables have been created, then + // `value` can't refer to any of them. =) So we can just return it. + if region_vars.is_empty() { + return Ok(value); + } + + let mut fudger = RegionFudger { + infcx: self, + region_vars: ®ion_vars, + origin: origin + }; + + Ok(value.fold_with(&mut fudger)) + } +} + +pub struct RegionFudger<'a, 'gcx: 'a+'tcx, 'tcx: 'a> { + infcx: &'a InferCtxt<'a, 'gcx, 'tcx>, + region_vars: &'a Vec, + origin: &'a RegionVariableOrigin, +} + +impl<'a, 'gcx, 'tcx> TypeFolder<'gcx, 'tcx> for RegionFudger<'a, 'gcx, 'tcx> { + fn tcx<'b>(&'b self) -> TyCtxt<'b, 'gcx, 'tcx> { + self.infcx.tcx + } + + fn fold_region(&mut self, r: &'tcx ty::Region) -> &'tcx ty::Region { + match *r { + ty::ReVar(v) if self.region_vars.contains(&v) => { + self.infcx.next_region_var(self.origin.clone()) + } + _ => { + r + } + } + } +} diff --git a/src/librustc/infer/mod.rs b/src/librustc/infer/mod.rs index 21820ca071921..8aac950b778bb 100644 --- a/src/librustc/infer/mod.rs +++ b/src/librustc/infer/mod.rs @@ -50,6 +50,7 @@ mod bivariate; mod combine; mod equate; pub mod error_reporting; +mod fudge; mod glb; mod higher_ranked; pub mod lattice; @@ -986,49 +987,6 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { r } - /// Execute `f` and commit only the region bindings if successful. - /// The function f must be very careful not to leak any non-region - /// variables that get created. - pub fn commit_regions_if_ok(&self, f: F) -> Result where - F: FnOnce() -> Result - { - debug!("commit_regions_if_ok()"); - let CombinedSnapshot { projection_cache_snapshot, - type_snapshot, - int_snapshot, - float_snapshot, - region_vars_snapshot, - obligations_in_snapshot } = self.start_snapshot(); - - let r = self.commit_if_ok(|_| f()); - - debug!("commit_regions_if_ok: rolling back everything but regions"); - - assert!(!self.obligations_in_snapshot.get()); - self.obligations_in_snapshot.set(obligations_in_snapshot); - - // Roll back any non-region bindings - they should be resolved - // inside `f`, with, e.g. `resolve_type_vars_if_possible`. - self.projection_cache - .borrow_mut() - .rollback_to(projection_cache_snapshot); - self.type_variables - .borrow_mut() - .rollback_to(type_snapshot); - self.int_unification_table - .borrow_mut() - .rollback_to(int_snapshot); - self.float_unification_table - .borrow_mut() - .rollback_to(float_snapshot); - - // Commit region vars that may escape through resolved types. - self.region_vars - .commit(region_vars_snapshot); - - r - } - /// Execute `f` then unroll any bindings it creates pub fn probe(&self, f: F) -> R where F: FnOnce(&CombinedSnapshot) -> R, diff --git a/src/librustc/infer/type_variable.rs b/src/librustc/infer/type_variable.rs index 3856ea420b0f3..804765ec8811e 100644 --- a/src/librustc/infer/type_variable.rs +++ b/src/librustc/infer/type_variable.rs @@ -184,6 +184,10 @@ impl<'tcx> TypeVariableTable<'tcx> { v } + pub fn num_vars(&self) -> usize { + self.values.len() + } + pub fn root_var(&mut self, vid: ty::TyVid) -> ty::TyVid { self.eq_relations.find(vid) } diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index d8314bd6c2aed..26778c83c3a06 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -86,7 +86,8 @@ use fmt_macros::{Parser, Piece, Position}; use hir::def::{Def, CtorKind, PathResolution}; use hir::def_id::{DefId, LOCAL_CRATE}; use hir::pat_util; -use rustc::infer::{self, InferCtxt, InferOk, TypeOrigin, TypeTrace, type_variable}; +use rustc::infer::{self, InferCtxt, InferOk, RegionVariableOrigin, + TypeOrigin, TypeTrace, type_variable}; use rustc::ty::subst::{Kind, Subst, Substs}; use rustc::traits::{self, Reveal}; use rustc::ty::{ParamTy, ParameterEnvironment}; @@ -2752,7 +2753,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { formal_args: &[Ty<'tcx>]) -> Vec> { let expected_args = expected_ret.only_has_type(self).and_then(|ret_ty| { - self.commit_regions_if_ok(|| { + self.fudge_regions_if_ok(&RegionVariableOrigin::Coercion(call_span), || { // Attempt to apply a subtyping relationship between the formal // return type (likely containing type variables if the function // is polymorphic) and the expected return type. diff --git a/src/librustc_typeck/check/regionck.rs b/src/librustc_typeck/check/regionck.rs index 6f6538254c46b..3d2277134bb9b 100644 --- a/src/librustc_typeck/check/regionck.rs +++ b/src/librustc_typeck/check/regionck.rs @@ -1149,7 +1149,7 @@ impl<'a, 'gcx, 'tcx> RegionCtxt<'a, 'gcx, 'tcx> { autoderefs: usize, autoref: &adjustment::AutoBorrow<'tcx>) { - debug!("link_autoref(autoref={:?})", autoref); + debug!("link_autoref(autoderefs={}, autoref={:?})", autoderefs, autoref); let mc = mc::MemCategorizationContext::new(self); let expr_cmt = ignore_err!(mc.cat_expr_autoderefd(expr, autoderefs)); debug!("expr_cmt={:?}", expr_cmt); diff --git a/src/test/run-pass/issue-37655.rs b/src/test/run-pass/issue-37655.rs new file mode 100644 index 0000000000000..d229bcacc501a --- /dev/null +++ b/src/test/run-pass/issue-37655.rs @@ -0,0 +1,46 @@ +// Copyright 2016 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 #37655. The problem was a false edge created by +// coercion that wound up requiring that `'a` (in `split()`) outlive +// `'b`, which shouldn't be necessary. + +#![allow(warnings)] + +trait SliceExt { + type Item; + + fn get_me(&self, index: I) -> &I::Output + where I: SliceIndex; +} + +impl SliceExt for [T] { + type Item = T; + + fn get_me(&self, index: I) -> &I::Output + where I: SliceIndex + { + panic!() + } +} + +pub trait SliceIndex { + type Output: ?Sized; +} + +impl SliceIndex for usize { + type Output = T; +} + +fn foo<'a, 'b>(split: &'b [&'a [u8]]) -> &'a [u8] { + split.get_me(0) +} + +fn main() { }