Skip to content

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

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

Merged
merged 2 commits into from
Nov 30, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Next Next commit
avoid type-live-for-region obligations on dummy nodes
Type-live-for-region obligations on DUMMY_NODE_ID cause an ICE, and it
turns out that in the few cases they are needed, these obligations are not
needed anyway because they are verified elsewhere.

Fixes #46069.
  • Loading branch information
arielb1 committed Nov 25, 2017
commit d049e5d19e4058dfdbf1ce54770a0e3ca36e5dd3
1 change: 1 addition & 0 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
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
20 changes: 15 additions & 5 deletions src/librustc/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,7 @@ 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();
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 @@ -583,9 +583,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 +596,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
2 changes: 1 addition & 1 deletion src/librustc/traits/specialize/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ 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();
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();
}