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

avoid type-live-for-region obligations on dummy nodes #46226

Merged
merged 2 commits into from
Nov 30, 2017
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
13 changes: 1 addition & 12 deletions src/librustc/infer/outlives/obligations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ impl<'cx, 'gcx, 'tcx> InferCtxt<'cx, 'gcx, 'tcx> {
body_id: ast::NodeId,
obligation: RegionObligation<'tcx>,
) {
debug!("register_region_obligation({:?}, {:?})", body_id, obligation);
self.region_obligations
.borrow_mut()
.push((body_id, obligation));
Expand Down Expand Up @@ -180,18 +181,6 @@ impl<'cx, 'gcx, 'tcx> InferCtxt<'cx, 'gcx, 'tcx> {
TypeOutlives::new(self, region_bound_pairs, implicit_region_bound, param_env);
outlives.type_must_outlive(origin, ty, region);
}

/// Ignore the region obligations, not bothering to prove
/// them. This function should not really exist; it is used to
/// accommodate some older code for the time being.
pub fn ignore_region_obligations(&self) {
assert!(
!self.in_snapshot.get(),
"cannot ignore registered region obligations in a snapshot"
);

self.region_obligations.borrow_mut().clear();
}
}

#[must_use] // you ought to invoke `into_accrued_obligations` when you are done =)
Expand Down
64 changes: 47 additions & 17 deletions src/librustc/traits/fulfill.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,19 @@ pub struct FulfillmentContext<'tcx> {
// A list of all obligations that have been registered with this
// fulfillment context.
predicates: ObligationForest<PendingPredicateObligation<'tcx>>,
// Should this fulfillment context register type-lives-for-region
// obligations on its parent infcx? In some cases, region
// obligations are either already known to hold (normalization) or
// hopefully verifed elsewhere (type-impls-bound), and therefore
// should not be checked.
//
// Note that if we are normalizing a type that we already
// know is well-formed, there should be no harm setting this
// to true - all the region variables should be determinable
// using the RFC 447 rules, which don't depend on
// type-lives-for-region constraints, and because the type
// is well-formed, the constraints should hold.
register_region_obligations: bool,
}

#[derive(Clone, Debug)]
Expand All @@ -59,6 +72,14 @@ impl<'a, 'gcx, 'tcx> FulfillmentContext<'tcx> {
pub fn new() -> FulfillmentContext<'tcx> {
FulfillmentContext {
predicates: ObligationForest::new(),
register_region_obligations: true
}
}

pub fn new_ignoring_regions() -> FulfillmentContext<'tcx> {
FulfillmentContext {
predicates: ObligationForest::new(),
register_region_obligations: false
}
}

Expand Down Expand Up @@ -191,7 +212,10 @@ impl<'a, 'gcx, 'tcx> FulfillmentContext<'tcx> {
debug!("select: starting another iteration");

// Process pending obligations.
let outcome = self.predicates.process_obligations(&mut FulfillProcessor { selcx });
let outcome = self.predicates.process_obligations(&mut FulfillProcessor {
selcx,
register_region_obligations: self.register_region_obligations
});
debug!("select: outcome={:?}", outcome);

// FIXME: if we kept the original cache key, we could mark projection
Expand Down Expand Up @@ -220,6 +244,7 @@ impl<'a, 'gcx, 'tcx> FulfillmentContext<'tcx> {

struct FulfillProcessor<'a, 'b: 'a, 'gcx: 'tcx, 'tcx: 'b> {
selcx: &'a mut SelectionContext<'b, 'gcx, 'tcx>,
register_region_obligations: bool
}

impl<'a, 'b, 'gcx, 'tcx> ObligationProcessor for FulfillProcessor<'a, 'b, 'gcx, 'tcx> {
Expand All @@ -230,7 +255,7 @@ impl<'a, 'b, 'gcx, 'tcx> ObligationProcessor for FulfillProcessor<'a, 'b, 'gcx,
obligation: &mut Self::Obligation)
-> Result<Option<Vec<Self::Obligation>>, Self::Error>
{
process_predicate(self.selcx, obligation)
process_predicate(self.selcx, obligation, self.register_region_obligations)
.map(|os| os.map(|os| os.into_iter().map(|o| PendingPredicateObligation {
obligation: o,
stalled_on: vec![]
Expand Down Expand Up @@ -269,7 +294,8 @@ fn trait_ref_type_vars<'a, 'gcx, 'tcx>(selcx: &mut SelectionContext<'a, 'gcx, 't
/// - `Err` if the predicate does not hold
fn process_predicate<'a, 'gcx, 'tcx>(
selcx: &mut SelectionContext<'a, 'gcx, 'tcx>,
pending_obligation: &mut PendingPredicateObligation<'tcx>)
pending_obligation: &mut PendingPredicateObligation<'tcx>,
register_region_obligations: bool)
-> Result<Option<Vec<PredicateObligation<'tcx>>>,
FulfillmentErrorCode<'tcx>>
{
Expand Down Expand Up @@ -391,26 +417,30 @@ fn process_predicate<'a, 'gcx, 'tcx>(
// `for<'a> T: 'a where 'a not in T`, which we can treat as `T: 'static`.
Some(t_a) => {
let r_static = selcx.tcx().types.re_static;
selcx.infcx().register_region_obligation(
obligation.cause.body_id,
RegionObligation {
sup_type: t_a,
sub_region: r_static,
cause: obligation.cause.clone(),
});
if register_region_obligations {
selcx.infcx().register_region_obligation(
obligation.cause.body_id,
RegionObligation {
sup_type: t_a,
sub_region: r_static,
cause: obligation.cause.clone(),
});
}
Ok(Some(vec![]))
}
}
}
// If there aren't, register the obligation.
Some(ty::OutlivesPredicate(t_a, r_b)) => {
selcx.infcx().register_region_obligation(
obligation.cause.body_id,
RegionObligation {
sup_type: t_a,
sub_region: r_b,
cause: obligation.cause.clone()
});
if register_region_obligations {
selcx.infcx().register_region_obligation(
obligation.cause.body_id,
RegionObligation {
sup_type: t_a,
sub_region: r_b,
cause: obligation.cause.clone()
});
}
Ok(Some(vec![]))
}
}
Expand Down
51 changes: 35 additions & 16 deletions src/librustc/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,10 @@ pub fn type_known_to_meet_bound<'a, 'gcx, 'tcx>(infcx: &InferCtxt<'a, 'gcx, 'tcx
// this function's result remains infallible, we must confirm
// that guess. While imperfect, I believe this is sound.

let mut fulfill_cx = FulfillmentContext::new();
// The handling of regions in this area of the code is terrible,
// see issue #29149. We should be able to improve on this with
// NLL.
let mut fulfill_cx = FulfillmentContext::new_ignoring_regions();

// We can use a dummy node-id here because we won't pay any mind
// to region obligations that arise (there shouldn't really be any
Expand Down Expand Up @@ -511,8 +514,24 @@ pub fn normalize_param_env_or_error<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
unnormalized_env.reveal);

tcx.infer_ctxt().enter(|infcx| {
let predicates = match fully_normalize(
// FIXME. We should really... do something with these region
// obligations. But this call just continues the older
// behavior (i.e., doesn't cause any new bugs), and it would
// take some further refactoring to actually solve them. In
// particular, we would have to handle implied bounds
// properly, and that code is currently largely confined to
// regionck (though I made some efforts to extract it
// out). -nmatsakis
//
// @arielby: In any case, these obligations are checked
// by wfcheck anyway, so I'm not sure we have to check
// them here too, and we will remove this function when
// we move over to lazy normalization *anyway*.
let fulfill_cx = FulfillmentContext::new_ignoring_regions();

let predicates = match fully_normalize_with_fulfillcx(
&infcx,
fulfill_cx,
cause,
elaborated_env,
// You would really want to pass infcx.param_env.caller_bounds here,
Expand All @@ -537,16 +556,6 @@ pub fn normalize_param_env_or_error<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
let region_scope_tree = region::ScopeTree::default();
let free_regions = FreeRegionMap::new();

// FIXME. We should really... do something with these region
// obligations. But this call just continues the older
// behavior (i.e., doesn't cause any new bugs), and it would
// take some further refactoring to actually solve them. In
// particular, we would have to handle implied bounds
// properly, and that code is currently largely confined to
// regionck (though I made some efforts to extract it
// out). -nmatsakis
let _ = infcx.ignore_region_obligations();

infcx.resolve_regions_and_report_errors(region_context, &region_scope_tree, &free_regions);
let predicates = match infcx.fully_resolve(&predicates) {
Ok(predicates) => predicates,
Expand Down Expand Up @@ -583,9 +592,6 @@ pub fn fully_normalize<'a, 'gcx, 'tcx, T>(infcx: &InferCtxt<'a, 'gcx, 'tcx>,
-> Result<T, Vec<FulfillmentError<'tcx>>>
where T : TypeFoldable<'tcx>
{
debug!("fully_normalize(value={:?})", value);

let selcx = &mut SelectionContext::new(infcx);
// FIXME (@jroesch) ISSUE 26721
// I'm not sure if this is a bug or not, needs further investigation.
// It appears that by reusing the fulfillment_cx here we incur more
Expand All @@ -599,8 +605,21 @@ pub fn fully_normalize<'a, 'gcx, 'tcx, T>(infcx: &InferCtxt<'a, 'gcx, 'tcx>,
//
// I think we should probably land this refactor and then come
// back to this is a follow-up patch.
let mut fulfill_cx = FulfillmentContext::new();
let fulfillcx = FulfillmentContext::new();
fully_normalize_with_fulfillcx(infcx, fulfillcx, cause, param_env, value)
}

pub fn fully_normalize_with_fulfillcx<'a, 'gcx, 'tcx, T>(
infcx: &InferCtxt<'a, 'gcx, 'tcx>,
mut fulfill_cx: FulfillmentContext<'tcx>,
cause: ObligationCause<'tcx>,
param_env: ty::ParamEnv<'tcx>,
value: &T)
-> Result<T, Vec<FulfillmentError<'tcx>>>
where T : TypeFoldable<'tcx>
{
debug!("fully_normalize_with_fulfillcx(value={:?})", value);
let selcx = &mut SelectionContext::new(infcx);
let Normalized { value: normalized_value, obligations } =
project::normalize(selcx, param_env, cause, value);
debug!("fully_normalize: normalized_value={:?} obligations={:?}",
Expand Down
13 changes: 12 additions & 1 deletion src/librustc/traits/specialize/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,18 @@ fn fulfill_implication<'a, 'gcx, 'tcx>(infcx: &InferCtxt<'a, 'gcx, 'tcx>,
// (which are packed up in penv)

infcx.save_and_restore_in_snapshot_flag(|infcx| {
let mut fulfill_cx = FulfillmentContext::new();
// If we came from `translate_substs`, we already know that the
// predicates for our impl hold (after all, we know that a more
// specialized impl holds, so our impl must hold too), and
// we only want to process the projections to determine the
// the types in our substs using RFC 447, so we can safely
// ignore region obligations, which allows us to avoid threading
// a node-id to assign them with.
//
// If we came from specialization graph construction, then
// we already make a mockery out of the region system, so
// why not ignore them a bit earlier?
let mut fulfill_cx = FulfillmentContext::new_ignoring_regions();
for oblig in obligations.into_iter() {
fulfill_cx.register_predicate_obligation(&infcx, oblig);
}
Expand Down
15 changes: 11 additions & 4 deletions src/librustc/traits/structural_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

use traits;
use traits::project::Normalized;
use ty::{Lift, TyCtxt};
use ty::{self, Lift, TyCtxt};
use ty::fold::{TypeFoldable, TypeFolder, TypeVisitor};

use std::fmt;
Expand All @@ -28,9 +28,16 @@ impl<'tcx, T: fmt::Debug> fmt::Debug for Normalized<'tcx, T> {

impl<'tcx, O: fmt::Debug> fmt::Debug for traits::Obligation<'tcx, O> {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "Obligation(predicate={:?},depth={})",
self.predicate,
self.recursion_depth)
if ty::tls::with(|tcx| tcx.sess.verbose()) {
write!(f, "Obligation(predicate={:?},cause={:?},depth={})",
self.predicate,
self.cause,
self.recursion_depth)
} else {
write!(f, "Obligation(predicate={:?},depth={})",
self.predicate,
self.recursion_depth)
}
}
}

Expand Down
13 changes: 10 additions & 3 deletions src/librustc_mir/transform/nll/constraint_generation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,10 +152,17 @@ impl<'cx, 'gcx, 'tcx> ConstraintGeneration<'cx, 'gcx, 'tcx> {
// associated types and parameters). We need to normalize
// associated types here and possibly recursively process.
for ty in dtorck_types {
// FIXME -- I think that this may disregard some region obligations
// or something. Do we care? -nmatsakis
let cause = ObligationCause::dummy();
match traits::fully_normalize(self.infcx, cause, self.param_env, &ty) {
// We know that our original `dropped_ty` is well-formed,
// so region obligations resulting from this normalization
// should always hold.
//
// Therefore we ignore them instead of trying to match
// them up with a location.
let fulfillcx = traits::FulfillmentContext::new_ignoring_regions();
match traits::fully_normalize_with_fulfillcx(
self.infcx, fulfillcx, cause, self.param_env, &ty
) {
Ok(ty) => match ty.sty {
ty::TyParam(..) | ty::TyProjection(..) | ty::TyAnon(..) => {
self.add_regular_live_constraint(ty, location);
Expand Down
32 changes: 32 additions & 0 deletions src/test/run-pass/issue-46069.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// Copyright 2017 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 std::iter::{Fuse, Cloned};
use std::slice::Iter;

struct Foo<'a, T: 'a>(&'a T);
impl<'a, T: 'a> Copy for Foo<'a, T> {}
impl<'a, T: 'a> Clone for Foo<'a, T> {
fn clone(&self) -> Self { *self }
}

fn copy_ex() {
let s = 2;
let k1 = || s;
let upvar = Foo(&k1);
let k = || upvar;
k();
}

fn main() {
let _f = 0 as *mut <Fuse<Cloned<Iter<u8>>> as Iterator>::Item;

copy_ex();
}