Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

introduce a fudge_regions_if_ok to address false region edges #37659

Merged
merged 1 commit into from
Nov 12, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
137 changes: 137 additions & 0 deletions src/librustc/infer/fudge.rs
Original file line number Diff line number Diff line change
@@ -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 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, 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<T> fn(T) -> Option<T>`. 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<?T>`, 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<?T`) -- and remember that
/// after the snapshot is popped, the variable `?T` is no longer
/// unified.
///
/// Assumptions:
/// - no new type variables are created during `f()` (asserted
/// below); this simplifies our logic since we don't have to
/// check for escaping type variables
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will break down once lazy normalization happens, right? Not that it blocks the PR.

pub fn fudge_regions_if_ok<T, E, F>(&self,
origin: &RegionVariableOrigin,
f: F) -> Result<T, E> where
F: FnOnce() -> Result<T, E>,
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: &region_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<ty::RegionVid>,
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
}
}
}
}
44 changes: 1 addition & 43 deletions src/librustc/infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ mod bivariate;
mod combine;
mod equate;
pub mod error_reporting;
mod fudge;
mod glb;
mod higher_ranked;
pub mod lattice;
Expand Down Expand Up @@ -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<T, E, F>(&self, f: F) -> Result<T, E> where
F: FnOnce() -> Result<T, E>
{
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<R, F>(&self, f: F) -> R where
F: FnOnce(&CombinedSnapshot) -> R,
Expand Down
4 changes: 4 additions & 0 deletions src/librustc/infer/type_variable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
5 changes: 3 additions & 2 deletions src/librustc_typeck/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -2752,7 +2753,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
formal_args: &[Ty<'tcx>])
-> Vec<Ty<'tcx>> {
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.
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_typeck/check/regionck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
46 changes: 46 additions & 0 deletions src/test/run-pass/issue-37655.rs
Original file line number Diff line number Diff line change
@@ -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 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, 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<T> {
type Item;

fn get_me<I>(&self, index: I) -> &I::Output
where I: SliceIndex<Self::Item>;
}

impl<T> SliceExt<T> for [T] {
type Item = T;

fn get_me<I>(&self, index: I) -> &I::Output
where I: SliceIndex<T>
{
panic!()
}
}

pub trait SliceIndex<T> {
type Output: ?Sized;
}

impl<T> SliceIndex<T> for usize {
type Output = T;
}

fn foo<'a, 'b>(split: &'b [&'a [u8]]) -> &'a [u8] {
split.get_me(0)
}

fn main() { }