From 3ac976861d68c9e00bb844c0feadbe1ce882d5e6 Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Wed, 21 Jun 2017 14:50:43 +0300 Subject: [PATCH 1/2] avoid translating roots with predicates that do not hold Fixes #37725. --- src/librustc/traits/mod.rs | 7 +++++-- src/librustc_trans/collector.rs | 14 +++++--------- src/librustc_trans/trans_item.rs | 18 +++++++++++++++++- src/test/run-pass/issue-37725.rs | 14 ++++++++++++++ 4 files changed, 41 insertions(+), 12 deletions(-) create mode 100644 src/test/run-pass/issue-37725.rs diff --git a/src/librustc/traits/mod.rs b/src/librustc/traits/mod.rs index e9196cd1243..d5d92cfcc3e 100644 --- a/src/librustc/traits/mod.rs +++ b/src/librustc/traits/mod.rs @@ -597,7 +597,7 @@ pub fn normalize_and_test_predicates<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, debug!("normalize_and_test_predicates(predicates={:?})", predicates); - tcx.infer_ctxt().enter(|infcx| { + let result = tcx.infer_ctxt().enter(|infcx| { let param_env = ty::ParamEnv::empty(Reveal::All); let mut selcx = SelectionContext::new(&infcx); let mut fulfill_cx = FulfillmentContext::new(); @@ -613,7 +613,10 @@ pub fn normalize_and_test_predicates<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, } fulfill_cx.select_all_or_error(&infcx).is_ok() - }) + }); + debug!("normalize_and_test_predicates(predicates={:?}) = {:?}", + predicates, result); + result } /// Given a trait `trait_ref`, iterates the vtable entries diff --git a/src/librustc_trans/collector.rs b/src/librustc_trans/collector.rs index d723cf32571..3536dcf16e5 100644 --- a/src/librustc_trans/collector.rs +++ b/src/librustc_trans/collector.rs @@ -195,7 +195,7 @@ use rustc::hir::map as hir_map; use rustc::hir::def_id::DefId; use rustc::middle::lang_items::{ExchangeMallocFnLangItem}; use rustc::traits; -use rustc::ty::subst::{Substs, Subst}; +use rustc::ty::subst::Substs; use rustc::ty::{self, TypeFoldable, TyCtxt}; use rustc::ty::adjustment::CustomCoerceUnsized; use rustc::mir::{self, Location}; @@ -304,6 +304,7 @@ fn collect_roots<'a, 'tcx>(scx: &SharedCrateContext<'a, 'tcx>, scx.tcx().hir.krate().visit_all_item_likes(&mut visitor); } + roots.retain(|root| root.is_instantiable(scx.tcx())); roots } @@ -937,14 +938,9 @@ fn create_trans_items_for_default_impls<'a, 'tcx>(scx: &SharedCrateContext<'a, ' let instance = monomorphize::resolve(scx, method.def_id, callee_substs); - let predicates = tcx.predicates_of(instance.def_id()).predicates - .subst(tcx, instance.substs); - if !traits::normalize_and_test_predicates(tcx, predicates) { - continue; - } - - if should_trans_locally(tcx, &instance) { - output.push(create_fn_trans_item(instance)); + let trans_item = create_fn_trans_item(instance); + if trans_item.is_instantiable(tcx) && should_trans_locally(tcx, &instance) { + output.push(trans_item); } } } diff --git a/src/librustc_trans/trans_item.rs b/src/librustc_trans/trans_item.rs index 0dc2bc85e30..6e8b2e0a2a6 100644 --- a/src/librustc_trans/trans_item.rs +++ b/src/librustc_trans/trans_item.rs @@ -25,8 +25,9 @@ use llvm; use monomorphize::Instance; use rustc::hir; use rustc::hir::def_id::DefId; +use rustc::traits; use rustc::ty::{self, Ty, TyCtxt, TypeFoldable}; -use rustc::ty::subst::Substs; +use rustc::ty::subst::{Subst, Substs}; use syntax::ast::{self, NodeId}; use syntax::attr; use syntax_pos::Span; @@ -250,6 +251,21 @@ impl<'a, 'tcx> TransItem<'tcx> { } } + /// Returns whether this instance is instantiable - whether it has no unsatisfied + /// predicates. + pub fn is_instantiable(&self, tcx: TyCtxt<'a, 'tcx, 'tcx>) -> bool { + debug!("is_instantiable({:?})", self); + let (def_id, substs) = match *self { + TransItem::Fn(ref instance) => (instance.def_id(), instance.substs), + TransItem::Static(node_id) => (tcx.hir.local_def_id(node_id), Substs::empty()), + // global asm never has predicates + TransItem::GlobalAsm(..) => return true + }; + + let predicates = tcx.predicates_of(def_id).predicates.subst(tcx, substs); + traits::normalize_and_test_predicates(tcx, predicates) + } + pub fn to_string(&self, tcx: TyCtxt<'a, 'tcx, 'tcx>) -> String { let hir_map = &tcx.hir; diff --git a/src/test/run-pass/issue-37725.rs b/src/test/run-pass/issue-37725.rs new file mode 100644 index 00000000000..5ed1295c85c --- /dev/null +++ b/src/test/run-pass/issue-37725.rs @@ -0,0 +1,14 @@ +// 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. + +pub fn foo<'a>(s: &'a mut ()) where &'a mut (): Clone { + s.clone(); +} +fn main() {} From a6ca302097ff30c0eb7746c5ef642fa4af4b6285 Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Wed, 28 Jun 2017 23:50:24 +0300 Subject: [PATCH 2/2] add comments --- src/librustc_trans/collector.rs | 4 ++++ src/librustc_trans/trans_item.rs | 23 +++++++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/src/librustc_trans/collector.rs b/src/librustc_trans/collector.rs index 3536dcf16e5..55a2c281403 100644 --- a/src/librustc_trans/collector.rs +++ b/src/librustc_trans/collector.rs @@ -304,7 +304,11 @@ fn collect_roots<'a, 'tcx>(scx: &SharedCrateContext<'a, 'tcx>, scx.tcx().hir.krate().visit_all_item_likes(&mut visitor); } + // We can only translate items that are instantiable - items all of + // whose predicates hold. Luckily, items that aren't instantiable + // can't actually be used, so we can just skip translating them. roots.retain(|root| root.is_instantiable(scx.tcx())); + roots } diff --git a/src/librustc_trans/trans_item.rs b/src/librustc_trans/trans_item.rs index 6e8b2e0a2a6..192e23a66a1 100644 --- a/src/librustc_trans/trans_item.rs +++ b/src/librustc_trans/trans_item.rs @@ -253,6 +253,29 @@ impl<'a, 'tcx> TransItem<'tcx> { /// Returns whether this instance is instantiable - whether it has no unsatisfied /// predicates. + /// + /// In order to translate an item, all of its predicates must hold, because + /// otherwise the item does not make sense. Type-checking ensures that + /// the predicates of every item that is *used by* a valid item *do* + /// hold, so we can rely on that. + /// + /// However, we translate collector roots (reachable items) and functions + /// in vtables when they are seen, even if they are not used, and so they + /// might not be instantiable. For example, a programmer can define this + /// public function: + /// + /// pub fn foo<'a>(s: &'a mut ()) where &'a mut (): Clone { + /// <&mut () as Clone>::clone(&s); + /// } + /// + /// That function can't be translated, because the method `<&mut () as Clone>::clone` + /// does not exist. Luckily for us, that function can't ever be used, + /// because that would require for `&'a mut (): Clone` to hold, so we + /// can just not emit any code, or even a linker reference for it. + /// + /// Similarly, if a vtable method has such a signature, and therefore can't + /// be used, we can just not emit it and have a placeholder (a null pointer, + /// which will never be accessed) in its place. pub fn is_instantiable(&self, tcx: TyCtxt<'a, 'tcx, 'tcx>) -> bool { debug!("is_instantiable({:?})", self); let (def_id, substs) = match *self {