From d234bf96991d4d296bbdf77a2256c6cac70e21ef Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Wed, 29 Jun 2016 19:14:01 +0200 Subject: [PATCH 1/4] Avoid ICE'ing due to panic in `lookup_item_type` in Debug impls. --- src/librustc/mir/repr.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/librustc/mir/repr.rs b/src/librustc/mir/repr.rs index 03ae91fefb925..f713268069329 100644 --- a/src/librustc/mir/repr.rs +++ b/src/librustc/mir/repr.rs @@ -1016,7 +1016,7 @@ impl<'tcx> Debug for Rvalue<'tcx> { ppaux::parameterized(fmt, substs, variant_def.did, ppaux::Ns::Value, &[], |tcx| { - Some(tcx.lookup_item_type(variant_def.did).generics) + tcx.opt_lookup_item_type(variant_def.did).map(|t|t.generics) })?; match variant_def.kind() { @@ -1112,7 +1112,7 @@ impl<'tcx> Debug for Literal<'tcx> { Item { def_id, substs } => { ppaux::parameterized( fmt, substs, def_id, ppaux::Ns::Value, &[], - |tcx| Some(tcx.lookup_item_type(def_id).generics)) + |tcx| tcx.opt_lookup_item_type(def_id).map(|t|t.generics)) } Value { ref value } => { write!(fmt, "const ")?; From 1366cb75bf9eb451e0052cb9a4f5559b5eaa1863 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Wed, 29 Jun 2016 19:18:04 +0200 Subject: [PATCH 2/4] Provide more info when `TraitRef::to_predicate` hits escaping region bug. --- src/librustc/ty/mod.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs index 2826680926637..055b763dbd9a0 100644 --- a/src/librustc/ty/mod.rs +++ b/src/librustc/ty/mod.rs @@ -1069,7 +1069,9 @@ impl<'tcx> ToPredicate<'tcx> for TraitRef<'tcx> { // we're about to add a binder, so let's check that we don't // accidentally capture anything, or else that might be some // weird debruijn accounting. - assert!(!self.has_escaping_regions()); + if self.has_escaping_regions() { + bug!("TraitRef::to_predicate {:?} has escaping regions", self); + } ty::Predicate::Trait(ty::Binder(ty::TraitPredicate { trait_ref: self.clone() From e84b400f7a070b799fefc6e0f18c3597aa176934 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Wed, 29 Jun 2016 19:47:40 +0200 Subject: [PATCH 3/4] Fix ICE in issue #33364. Also, avoid redundant confirmation work in trans. The main idea here is that if there are no unification problems to solve, then trans does not need to do any confirmation of the impls that it selects for a trait, nor the associated items it projects for a trait. (In principle the above mentioned ICE should instead be solved in the long term via lazy normalization. But for now this is a simpler solution and a good idea in any case.) --- src/librustc/traits/project.rs | 28 ++++++++++++++++++++++++++++ src/librustc/traits/select.rs | 13 +++++++++++++ src/librustc/ty/mod.rs | 4 ++++ 3 files changed, 45 insertions(+) diff --git a/src/librustc/traits/project.rs b/src/librustc/traits/project.rs index 30faf1a5f8b91..5d7ab8ab952a5 100644 --- a/src/librustc/traits/project.rs +++ b/src/librustc/traits/project.rs @@ -1247,6 +1247,34 @@ fn confirm_callable_candidate<'cx, 'gcx, 'tcx>( fn_sig, flag); + if selcx.projection_mode().is_any() { + // Issue #33364: `ProjectionMode::Any` means trans, during + // which there is no (good) reason for confirmation to fail + // (because type check has already admitted the source code). + // + // The only reason trans needs to run confirm is finding + // solutions for outstanding unification variables. (For now, + // that empirically appears to be necessary in some cases). + // + // So: if we can prove there is no reason to confirm, skip it. + // + // (Skipping confirmation sidesteps ICE in #33364. Even if + // that were fixed by other means, skipping is still good + // since redundant confirmations wastes compile-time.) + + let unsolved_tys = trait_ref.substs.types.iter().any(|t| t.is_ty_var()); + if unsolved_tys || ret_type.region_depth() > 0 { + debug!("confirm_callable_candidate need confirm for \ + unsolved_tys: {} ret_type: {:?} region_depth {}", + unsolved_tys, ret_type, ret_type.region_depth()); + } else { + debug!("confirm_callable_candidate skip confirm for \ + trait_ref: {:?} ret_type: {:?}", + trait_ref, ret_type); + return Progress { ty: ret_type, obligations: vec![], cacheable: false, }; + } + } + let predicate = ty::Binder(ty::ProjectionPredicate { // (1) recreate binder here projection_ty: ty::ProjectionTy { trait_ref: trait_ref, diff --git a/src/librustc/traits/select.rs b/src/librustc/traits/select.rs index 7a20b43b8f2e6..17e52e19be1f6 100644 --- a/src/librustc/traits/select.rs +++ b/src/librustc/traits/select.rs @@ -2453,6 +2453,19 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { expected_trait_ref: ty::PolyTraitRef<'tcx>) -> Result<(), SelectionError<'tcx>> { + if self.projection_mode().is_any() { + // Issue #33364: `ProjectionMode::Any` means trans. This + // method is not looking up a vtable, so its feasible to + // skip confirmation. + // + // (Skipping confirmation sidesteps ICE in #33364. Even if + // that were fixed by other means, skipping is still good + // since redundant confirmations wastes compile-time.) + + debug!("confirm_poly_trait_refs skip confirm unconditionally"); + return Ok(()); + } + let origin = TypeOrigin::RelateOutputImplTypes(obligation_cause.span); let obligation_trait_ref = obligation_trait_ref.clone(); diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs index 055b763dbd9a0..7e1f148807137 100644 --- a/src/librustc/ty/mod.rs +++ b/src/librustc/ty/mod.rs @@ -563,6 +563,10 @@ pub struct TyS<'tcx> { region_depth: u32, } +impl<'tcx> TyS<'tcx> { + pub fn region_depth(&self) -> u32 { self.region_depth } +} + impl<'tcx> PartialEq for TyS<'tcx> { #[inline] fn eq(&self, other: &TyS<'tcx>) -> bool { From 99a44e653d0c0df83c9e1fd84fcfa5a285561d31 Mon Sep 17 00:00:00 2001 From: "Felix S. Klock II" Date: Thu, 30 Jun 2016 15:14:55 +0200 Subject: [PATCH 4/4] Regression tests. --- src/test/run-pass/issue-33364-reduced.rs | 55 +++++++++++++++++++ .../issue-33364-region-in-ret-type.rs | 25 +++++++++ 2 files changed, 80 insertions(+) create mode 100644 src/test/run-pass/issue-33364-reduced.rs create mode 100644 src/test/run-pass/issue-33364-region-in-ret-type.rs diff --git a/src/test/run-pass/issue-33364-reduced.rs b/src/test/run-pass/issue-33364-reduced.rs new file mode 100644 index 0000000000000..e168af1b13c97 --- /dev/null +++ b/src/test/run-pass/issue-33364-reduced.rs @@ -0,0 +1,55 @@ +// 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. + +// Issue #33364: This is a reduced version of the reported code that +// caused an ICE. The ICE was due to a confirmation step during trans +// attempting (and failing) to match up the actual argument type to +// the closure (in this case, `&u32`) with the Fn argument type, which +// is a projection of the form `>::Item`. + +use std::marker::PhantomData; + +trait Foo<'a> { + type Item; + fn consume(self, f: F) where F: Fn(Self::Item); +} +struct Consume(PhantomData); + +impl<'a, A:'a> Foo<'a> for Consume { + type Item = &'a A; + + fn consume(self, _f: F) where F: Fn(Self::Item) { + if blackbox() { + _f(any()); + } + } +} + +#[derive(Clone)] +struct Wrap { foo: T } + +impl Foo<'a>> Wrap { + fn consume(self, f: F) where F: for <'b> Fn(>::Item) { + self.foo.consume(f); + } +} + +fn main() { + // This works + Consume(PhantomData::).consume(|item| { let _a = item; }); + + // This used to not work (but would only be noticed if you call closure). + let _wrap = Wrap { foo: Consume(PhantomData::,) }; + _wrap.consume(|item| { let _a = item; }); +} + +pub static mut FLAG: bool = false; +fn blackbox() -> bool { unsafe { FLAG } } +fn any() -> T { loop { } } diff --git a/src/test/run-pass/issue-33364-region-in-ret-type.rs b/src/test/run-pass/issue-33364-region-in-ret-type.rs new file mode 100644 index 0000000000000..d423fca9d75b5 --- /dev/null +++ b/src/test/run-pass/issue-33364-region-in-ret-type.rs @@ -0,0 +1,25 @@ +// 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. + +// Issue #33364: While working on the hack/fix to workaround an ICE, I +// injected a new ICE that was not tested by our run-pass test suite +// (but was exposed by attempting to bootstrap the compiler itself, as +// well as our compile-fail tests). This test attempts to codify the +// problem I encountered at that time, as a run-pass test. + +#![feature(unboxed_closures)] +fn main() +{ + static X: u32 = 3; + fn lifetime_in_ret_type_alone<'a>() -> &'a u32 { &X } + fn apply_thunk>(t: T, _x: T::Output) { t(); } + + apply_thunk(lifetime_in_ret_type_alone, &3); +}