When testing whether a default method predicates are satisfiable,

combine normalization with this check so that we also skip the
default method if normalization fails. Fixes rust-lang#23485.

Author: nikomatsakis

src/librustc/middle/traits/mod.rs

@@ -39,6 +39,7 @@ pub use self::object_safety::is_object_safe;
 pub use self::object_safety::object_safety_violations;
 pub use self::object_safety::ObjectSafetyViolation;
 pub use self::object_safety::MethodViolationCode;
+pub use self::object_safety::is_vtable_safe_method;
 pub use self::select::SelectionContext;
 pub use self::select::SelectionCache;
 pub use self::select::{MethodMatchResult, MethodMatched, MethodAmbiguous, MethodDidNotMatch};

src/librustc/middle/traits/object_safety.rs

@@ -96,7 +96,7 @@ fn object_safety_violations_for_trait<'tcx>(tcx: &ty::ctxt<'tcx>,
         .flat_map(|item| {
             match *item {
                 ty::MethodTraitItem(ref m) => {
-                    object_safety_violations_for_method(tcx, trait_def_id, &**m)
+                    object_safety_violation_for_method(tcx, trait_def_id, &**m)
                         .map(|code| ObjectSafetyViolation::Method(m.clone(), code))
                         .into_iter()
                 }
@@ -193,17 +193,42 @@ fn generics_require_sized_self<'tcx>(tcx: &ty::ctxt<'tcx>,
         })
 }
 
-fn object_safety_violations_for_method<'tcx>(tcx: &ty::ctxt<'tcx>,
-                                             trait_def_id: ast::DefId,
-                                             method: &ty::Method<'tcx>)
-                                             -> Option<MethodViolationCode>
+/// Returns `Some(_)` if this method makes the containing trait not object safe.
+fn object_safety_violation_for_method<'tcx>(tcx: &ty::ctxt<'tcx>,
+                                            trait_def_id: ast::DefId,
+                                            method: &ty::Method<'tcx>)
+                                            -> Option<MethodViolationCode>
 {
     // Any method that has a `Self : Sized` requisite is otherwise
     // exempt from the regulations.
     if generics_require_sized_self(tcx, &method.generics, &method.predicates) {
         return None;
     }
 
+    virtual_call_violation_for_method(tcx, trait_def_id, method)
+}
+
+/// We say a method is *vtable safe* if it can be invoked on a trait
+/// object.  Note that object-safe traits can have some
+/// non-vtable-safe methods, so long as they require `Self:Sized` or
+/// otherwise ensure that they cannot be used when `Self=Trait`.
+pub fn is_vtable_safe_method<'tcx>(tcx: &ty::ctxt<'tcx>,
+                                   trait_def_id: ast::DefId,
+                                   method: &ty::Method<'tcx>)
+                                   -> bool
+{
+    virtual_call_violation_for_method(tcx, trait_def_id, method).is_none()
+}
+
+/// Returns `Some(_)` if this method cannot be called on a trait
+/// object; this does not necessarily imply that the enclosing trait
+/// is not object safe, because the method might have a where clause
+/// `Self:Sized`.
+fn virtual_call_violation_for_method<'tcx>(tcx: &ty::ctxt<'tcx>,
+                                           trait_def_id: ast::DefId,
+                                           method: &ty::Method<'tcx>)
+                                           -> Option<MethodViolationCode>
+{
     // The method's first parameter must be something that derefs (or
     // autorefs) to `&self`. For now, we only accept `self`, `&self`
     // and `Box<Self>`.

src/librustc_trans/trans/common.rs

@@ -1069,17 +1069,30 @@ 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
+/// Normalizes the predicates and checks whether they hold.  If this
+/// returns false, then either normalize encountered an error or one
+/// of the predicates did not hold. Used when creating vtables to
+/// check for unsatisfiable methods.
+pub fn normalize_and_test_predicates<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>,
+                                               predicates: Vec<ty::Predicate<'tcx>>)
+                                               -> bool
 {
-    debug!("predicates_hold(predicates={})",
+    debug!("normalize_and_test_predicates(predicates={})",
            predicates.repr(ccx.tcx()));
 
-    let infcx = infer::new_infer_ctxt(ccx.tcx());
+    let tcx = ccx.tcx();
+    let infcx = infer::new_infer_ctxt(tcx);
+    let typer = NormalizingClosureTyper::new(tcx);
+    let mut selcx = traits::SelectionContext::new(&infcx, &typer);
     let mut fulfill_cx = traits::FulfillmentContext::new();
+    let cause = traits::ObligationCause::dummy();
+    let traits::Normalized { value: predicates, obligations } =
+        traits::normalize(&mut selcx, cause.clone(), &predicates);
+    for obligation in obligations {
+        fulfill_cx.register_predicate_obligation(&infcx, obligation);
+    }
     for predicate in predicates {
-        let obligation = traits::Obligation::new(traits::ObligationCause::dummy(), predicate);
+        let obligation = traits::Obligation::new(cause.clone(), predicate);
         fulfill_cx.register_predicate_obligation(&infcx, obligation);
     }
     drain_fulfillment_cx(&infcx, &mut fulfill_cx, &()).is_ok()

src/librustc_trans/trans/meth.rs

@@ -13,7 +13,7 @@ use back::abi;
 use back::link;
 use llvm::{ValueRef, get_param};
 use metadata::csearch;
-use middle::subst::Substs;
+use middle::subst::{Subst, Substs};
 use middle::subst::VecPerParamSpace;
 use middle::subst;
 use middle::traits;
@@ -784,6 +784,7 @@ fn emit_vtable_methods<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>,
 
     ty::populate_implementations_for_trait_if_necessary(tcx, trt_id);
 
+    let nullptr = C_null(Type::nil(ccx).ptr_to());
     let trait_item_def_ids = ty::trait_item_def_ids(tcx, trt_id);
     trait_item_def_ids
         .iter()
@@ -809,6 +810,12 @@ fn emit_vtable_methods<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>,
             };
             let name = trait_method_type.name;
 
+            // Some methods cannot be called on an object; skip those.
+            if !traits::is_vtable_safe_method(tcx, trt_id, &trait_method_type) {
+                debug!("emit_vtable_methods: not vtable safe");
+                return nullptr;
+            }
+
             debug!("emit_vtable_methods: trait_method_type={}",
                    trait_method_type.repr(tcx));
 
@@ -820,35 +827,17 @@ fn emit_vtable_methods<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>,
                 ty::TypeTraitItem(_) => ccx.sess().bug("should be a method, not assoc type")
             };
 
-            debug!("emit_vtable_methods: m={}",
+            debug!("emit_vtable_methods: impl_method_type={}",
                    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;
-            }
-
-            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;
-            }
-
             // 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()) {
+                let predicates = impl_method_type.predicates.predicates.subst(tcx, &substs);
+                if !normalize_and_test_predicates(ccx, predicates.into_vec()) {
                     debug!("emit_vtable_methods: predicates do not hold");
                     return nullptr;
                 }

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

@@ -0,0 +1,58 @@
+// Copyright 2014 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 for an ICE that occured when a default method implementation
+// was applied to a type that did not meet the prerequisites. The
+// problem occurred specifically because normalizing
+// `Self::Item::Target` was impossible in this case.
+
+use std::boxed::Box;
+use std::marker::Sized;
+use std::clone::Clone;
+use std::ops::Deref;
+use std::option::Option;
+use std::option::Option::{Some,None};
+
+trait Iterator {
+    type Item;
+
+    fn next(&mut self) -> Option<Self::Item>;
+
+    fn clone_first(mut self) -> Option<<Self::Item as Deref>::Target> where
+        Self: Sized,
+        Self::Item: Deref,
+        <Self::Item as Deref>::Target: Clone,
+    {
+        self.next().cloned()
+    }
+}
+
+struct Counter {
+    value: i32
+}
+
+struct Token {
+    value: i32
+}
+
+impl Iterator for Counter {
+    type Item = Token;
+
+    fn next(&mut self) -> Option<Token> {
+        let x = self.value;
+        self.value += 1;
+        Some(Token { value: x })
+    }
+}
+
+fn main() {
+    let mut x: Box<Iterator<Item=Token>> = Box::new(Counter { value: 22 });
+    assert_eq!(x.next().unwrap().value, 22);
+}