Skip default methods whose predicates are not satisfied when constructing vtables

src/librustc/util/ppaux.rs

@@ -1053,10 +1053,11 @@ impl<'tcx> Repr<'tcx> for ty::Variance {
 
 impl<'tcx> Repr<'tcx> for ty::Method<'tcx> {
     fn repr(&self, tcx: &ctxt<'tcx>) -> String {
-        format!("method(name: {}, generics: {}, fty: {}, \
+        format!("method(name: {}, generics: {}, predicates: {}, fty: {}, \
                  explicit_self: {}, vis: {}, def_id: {})",
                 self.name.repr(tcx),
                 self.generics.repr(tcx),
+                self.predicates.repr(tcx),
                 self.fty.repr(tcx),
                 self.explicit_self.repr(tcx),
                 self.vis.repr(tcx),

src/librustc_trans/trans/common.rs

@@ -46,6 +46,7 @@ use arena::TypedArena;
 use libc::{c_uint, c_char};
 use std::ffi::CString;
 use std::cell::{Cell, RefCell};
+use std::result::Result as StdResult;
 use std::vec::Vec;
 use syntax::ast::Ident;
 use syntax::ast;
@@ -1006,9 +1007,9 @@ pub fn expr_ty_adjusted<'blk, 'tcx>(bcx: &BlockS<'blk, 'tcx>, ex: &ast::Expr) ->
 /// do not (necessarily) resolve all nested obligations on the impl. Note that type check should
 /// guarantee to us that all nested obligations *could be* resolved if we wanted to.
 pub fn fulfill_obligation<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>,
-                                span: Span,
-                                trait_ref: ty::PolyTraitRef<'tcx>)
-                                -> traits::Vtable<'tcx, ()>
+                                    span: Span,
+                                    trait_ref: ty::PolyTraitRef<'tcx>)
+                                    -> traits::Vtable<'tcx, ()>
 {
     let tcx = ccx.tcx();
 
@@ -1067,7 +1068,7 @@ pub fn fulfill_obligation<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>,
     let vtable = selection.map_move_nested(|predicate| {
         fulfill_cx.register_predicate_obligation(&infcx, predicate);
     });
-    let vtable = drain_fulfillment_cx(span, &infcx, &mut fulfill_cx, &vtable);
+    let vtable = drain_fulfillment_cx_or_panic(span, &infcx, &mut fulfill_cx, &vtable);
 
     info!("Cache miss: {}", trait_ref.repr(ccx.tcx()));
     ccx.trait_cache().borrow_mut().insert(trait_ref,
@@ -1076,6 +1077,22 @@ pub fn fulfill_obligation<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>,
     vtable
 }
 
+pub fn predicates_hold<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>,
+                                 predicates: Vec<ty::Predicate<'tcx>>)
+                                 -> bool
+{
+    debug!("predicates_hold(predicates={})",
+           predicates.repr(ccx.tcx()));
+
+    let infcx = infer::new_infer_ctxt(ccx.tcx());
+    let mut fulfill_cx = traits::FulfillmentContext::new();
+    for predicate in predicates {
+        let obligation = traits::Obligation::new(traits::ObligationCause::dummy(), predicate);
+        fulfill_cx.register_predicate_obligation(&infcx, obligation);
+    }
+    drain_fulfillment_cx(DUMMY_SP, &infcx, &mut fulfill_cx, &()).is_ok()
+}
+
 pub struct NormalizingClosureTyper<'a,'tcx:'a> {
     param_env: ty::ParameterEnvironment<'a, 'tcx>
 }
@@ -1123,11 +1140,36 @@ impl<'a,'tcx> ty::ClosureTyper<'tcx> for NormalizingClosureTyper<'a,'tcx> {
     }
 }
 
+pub fn drain_fulfillment_cx_or_panic<'a,'tcx,T>(span: Span,
+                                                infcx: &infer::InferCtxt<'a,'tcx>,
+                                                fulfill_cx: &mut traits::FulfillmentContext<'tcx>,
+                                                result: &T)
+                                                -> T
+    where T : TypeFoldable<'tcx> + Repr<'tcx>
+{
+    match drain_fulfillment_cx(span, infcx, fulfill_cx, result) {
+        Ok(v) => v,
+        Err(errors) => {
+            infcx.tcx.sess.span_bug(
+                span,
+                &format!("Encountered errors `{}` fulfilling during trans",
+                         errors.repr(infcx.tcx)));
+        }
+    }
+}
+
+/// Finishes processes any obligations that remain in the fulfillment
+/// context, and then "freshens" and returns `result`. This is
+/// primarily used during normalization and other cases where
+/// processing the obligations in `fulfill_cx` may cause type
+/// inference variables that appear in `result` to be unified, and
+/// hence we need to process those obligations to get the complete
+/// picture of the type.
 pub fn drain_fulfillment_cx<'a,'tcx,T>(span: Span,
-                                   infcx: &infer::InferCtxt<'a,'tcx>,
-                                   fulfill_cx: &mut traits::FulfillmentContext<'tcx>,
-                                   result: &T)
-                                   -> T
+                                       infcx: &infer::InferCtxt<'a,'tcx>,
+                                       fulfill_cx: &mut traits::FulfillmentContext<'tcx>,
+                                       result: &T)
+                                       -> StdResult<T,Vec<traits::FulfillmentError<'tcx>>>
     where T : TypeFoldable<'tcx> + Repr<'tcx>
 {
     debug!("drain_fulfillment_cx(result={})",
@@ -1140,16 +1182,13 @@ pub fn drain_fulfillment_cx<'a,'tcx,T>(span: Span,
     match fulfill_cx.select_all_or_error(infcx, &typer) {
         Ok(()) => { }
         Err(errors) => {
+            // We always want to surface any overflow errors, no matter what.
             if errors.iter().all(|e| e.is_overflow()) {
-                // See Ok(None) case above.
                 infcx.tcx.sess.span_fatal(
                     span,
                     "reached the recursion limit during monomorphization");
             } else {
-                infcx.tcx.sess.span_bug(
-                    span,
-                    &format!("Encountered errors `{}` fulfilling during trans",
-                            errors.repr(infcx.tcx)));
+                return Err(errors);
             }
         }
     }
@@ -1159,7 +1198,7 @@ pub fn drain_fulfillment_cx<'a,'tcx,T>(span: Span,
     // sort of overkill because we do not expect there to be any
     // unbound type variables, hence no `TyFresh` types should ever be
     // inserted.
-    result.fold_with(&mut infcx.freshener())
+    Ok(result.fold_with(&mut infcx.freshener()))
 }
 
 // Key used to lookup values supplied for type parameters in an expr.

src/librustc_trans/trans/meth.rs

@@ -771,9 +771,15 @@ fn emit_vtable_methods<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>,
                                  impl_id: ast::DefId,
                                  substs: subst::Substs<'tcx>,
                                  param_substs: &'tcx subst::Substs<'tcx>)
-                                 -> Vec<ValueRef> {
+                                 -> Vec<ValueRef>
+{
     let tcx = ccx.tcx();
 
+    debug!("emit_vtable_methods(impl_id={}, substs={}, param_substs={})",
+           impl_id.repr(tcx),
+           substs.repr(tcx),
+           param_substs.repr(tcx));
+
     let trt_id = match ty::impl_trait_ref(tcx, impl_id) {
         Some(t_id) => t_id.def_id,
         None       => ccx.sess().bug("make_impl_vtable: don't know how to \
@@ -783,41 +789,82 @@ fn emit_vtable_methods<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>,
     ty::populate_implementations_for_trait_if_necessary(tcx, trt_id);
 
     let trait_item_def_ids = ty::trait_item_def_ids(tcx, trt_id);
-    trait_item_def_ids.iter().flat_map(|method_def_id| {
-        let method_def_id = method_def_id.def_id();
-        let name = ty::impl_or_trait_item(tcx, method_def_id).name();
-        // The substitutions we have are on the impl, so we grab
-        // the method type from the impl to substitute into.
-        let m_id = method_with_name(ccx, impl_id, name);
-        let ti = ty::impl_or_trait_item(tcx, m_id);
-        match ti {
-            ty::MethodTraitItem(m) => {
-                debug!("(making impl vtable) emitting method {} at subst {}",
-                       m.repr(tcx),
-                       substs.repr(tcx));
-                if m.generics.has_type_params(subst::FnSpace) ||
-                    ty::type_has_self(ty::mk_bare_fn(tcx, None, tcx.mk_bare_fn(m.fty.clone())))
-                {
-                    debug!("(making impl vtable) method has self or type \
-                            params: {}",
-                           token::get_name(name));
-                    Some(C_null(Type::nil(ccx).ptr_to())).into_iter()
-                } else {
-                    let fn_ref = trans_fn_ref_with_substs(
-                        ccx,
-                        m_id,
-                        ExprId(0),
-                        param_substs,
-                        substs.clone()).val;
-
-                    Some(fn_ref).into_iter()
-                }
+    trait_item_def_ids
+        .iter()
+
+        // Filter out the associated types.
+        .filter_map(|item_def_id| {
+            match *item_def_id {
+                ty::MethodTraitItemId(def_id) => Some(def_id),
+                ty::TypeTraitItemId(_) => None,
+            }
+        })
+
+        // Now produce pointers for each remaining method. If the
+        // method could never be called from this object, just supply
+        // null.
+        .map(|trait_method_def_id| {
+            debug!("emit_vtable_methods: trait_method_def_id={}",
+                   trait_method_def_id.repr(tcx));
+
+            let trait_method_type = match ty::impl_or_trait_item(tcx, trait_method_def_id) {
+                ty::MethodTraitItem(m) => m,
+                ty::TypeTraitItem(_) => ccx.sess().bug("should be a method, not assoc type")
+            };
+            let name = trait_method_type.name;
+
+            debug!("emit_vtable_methods: trait_method_type={}",
+                   trait_method_type.repr(tcx));
+
+            // The substitutions we have are on the impl, so we grab
+            // the method type from the impl to substitute into.
+            let impl_method_def_id = method_with_name(ccx, impl_id, name);
+            let impl_method_type = match ty::impl_or_trait_item(tcx, impl_method_def_id) {
+                ty::MethodTraitItem(m) => m,
+                ty::TypeTraitItem(_) => ccx.sess().bug("should be a method, not assoc type")
+            };
+
+            debug!("emit_vtable_methods: m={}",
+                   impl_method_type.repr(tcx));
+
+            let nullptr = C_null(Type::nil(ccx).ptr_to());
+
+            if impl_method_type.generics.has_type_params(subst::FnSpace) {
+                debug!("emit_vtable_methods: generic");
+                return nullptr;
             }
-            ty::TypeTraitItem(_) => {
-                None.into_iter()
+
+            let bare_fn_ty =
+                ty::mk_bare_fn(tcx, None, tcx.mk_bare_fn(impl_method_type.fty.clone()));
+            if ty::type_has_self(bare_fn_ty) {
+                debug!("emit_vtable_methods: type_has_self {}",
+                       bare_fn_ty.repr(tcx));
+                return nullptr;
             }
-        }
-    }).collect()
+
+            // If this is a default method, it's possible that it
+            // relies on where clauses that do not hold for this
+            // particular set of type parameters. Note that this
+            // method could then never be called, so we do not want to
+            // try and trans it, in that case. Issue #23435.
+            if ty::provided_source(tcx, impl_method_def_id).is_some() {
+                let predicates =
+                    monomorphize::apply_param_substs(tcx,
+                                                     &substs,
+                                                     &impl_method_type.predicates.predicates);
+                if !predicates_hold(ccx, predicates.into_vec()) {
+                    debug!("emit_vtable_methods: predicates do not hold");
+                    return nullptr;
+                }
+            }
+
+            trans_fn_ref_with_substs(ccx,
+                                     impl_method_def_id,
+                                     ExprId(0),
+                                     param_substs,
+                                     substs.clone()).val
+        })
+        .collect()
 }
 
 /// Generates the code to convert from a pointer (`Box<T>`, `&T`, etc) into an object

src/librustc_trans/trans/monomorphize.rs

@@ -339,7 +339,7 @@ pub fn normalize_associated_type<'tcx,T>(tcx: &ty::ctxt<'tcx>, value: &T) -> T
     for obligation in obligations {
         fulfill_cx.register_predicate_obligation(&infcx, obligation);
     }
-    let result = drain_fulfillment_cx(DUMMY_SP, &infcx, &mut fulfill_cx, &result);
+    let result = drain_fulfillment_cx_or_panic(DUMMY_SP, &infcx, &mut fulfill_cx, &result);
 
     result
 }

src/test/run-pass/issue-23435.rs

@@ -0,0 +1,35 @@
+// Copyright 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.
+
+// Test that we do not ICE when a default method implementation has
+// requirements (in this case, `Self : Baz`) that do not hold for some
+// specific impl (in this case, `Foo : Bar`). This causes problems
+// only when building a vtable, because that goes along and
+// instantiates all the methods, even those that could not otherwise
+// be called.
+
+struct Foo {
+    x: i32
+}
+
+trait Bar {
+    fn bar(&self) where Self : Baz { self.baz(); }
+}
+
+trait Baz {
+    fn baz(&self);
+}
+
+impl Bar for Foo {
+}
+
+fn main() {
+    let x: &Bar = &Foo { x: 22 };
+}