auto merge of #18749 : nikomatsakis/rust/builtin-bounds-like-other-traits, r=pcwalton

Treat builtin bounds like all other kinds of trait matches. Introduce a simple hashset in the fulfillment context to catch cases where we register the exact same obligation twice. This helps prevent duplicate error reports but also handles the recursive obligations created by builtin bounds.

r? @pcwalton 
cc @flaper87

Author: bors

src/librustc/middle/traits/fulfill.rs

@@ -11,6 +11,8 @@
 use middle::mem_categorization::Typer;
 use middle::ty;
 use middle::typeck::infer::InferCtxt;
+use std::collections::HashSet;
+use std::rc::Rc;
 use util::ppaux::Repr;
 
 use super::CodeAmbiguity;
@@ -30,6 +32,13 @@ use super::select::SelectionContext;
 /// method `select_all_or_error` can be used to report any remaining
 /// ambiguous cases as errors.
 pub struct FulfillmentContext<'tcx> {
+    // a simple cache that aims to cache *exact duplicate obligations*
+    // and avoid adding them twice. This serves a different purpose
+    // than the `SelectionCache`: it avoids duplicate errors and
+    // permits recursive obligations, which are often generated from
+    // traits like `Send` et al.
+    duplicate_set: HashSet<Rc<ty::TraitRef<'tcx>>>,
+
     // A list of all obligations that have been registered with this
     // fulfillment context.
     trait_obligations: Vec<Obligation<'tcx>>,
@@ -43,6 +52,7 @@ pub struct FulfillmentContext<'tcx> {
 impl<'tcx> FulfillmentContext<'tcx> {
     pub fn new() -> FulfillmentContext<'tcx> {
         FulfillmentContext {
+            duplicate_set: HashSet::new(),
             trait_obligations: Vec::new(),
             attempted_mark: 0,
         }
@@ -52,9 +62,13 @@ impl<'tcx> FulfillmentContext<'tcx> {
                                tcx: &ty::ctxt<'tcx>,
                                obligation: Obligation<'tcx>)
     {
-        debug!("register_obligation({})", obligation.repr(tcx));
-        assert!(!obligation.trait_ref.has_escaping_regions());
-        self.trait_obligations.push(obligation);
+        if self.duplicate_set.insert(obligation.trait_ref.clone()) {
+            debug!("register_obligation({})", obligation.repr(tcx));
+            assert!(!obligation.trait_ref.has_escaping_regions());
+            self.trait_obligations.push(obligation);
+        } else {
+            debug!("register_obligation({}) -- already seen, skip", obligation.repr(tcx));
+        }
     }
 
     pub fn select_all_or_error<'a>(&mut self,

src/librustc/middle/traits/select.rs

@@ -113,7 +113,7 @@ pub enum MethodMatchedData {
 /// candidate is one that might match or might not, depending on how
 /// type variables wind up being resolved. This only occurs during inference.
 ///
-/// For selection to suceed, there must be exactly one non-ambiguous
+/// For selection to succeed, there must be exactly one non-ambiguous
 /// candidate.  Usually, it is not possible to have more than one
 /// definitive candidate, due to the coherence rules. However, there is
 /// one case where it could occur: if there is a blanket impl for a
@@ -1149,24 +1149,12 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                                              candidates: &mut CandidateSet<'tcx>)
                                              -> Result<(),SelectionError<'tcx>>
     {
-        // FIXME -- To be more like a normal impl, we should just
-        // ignore the nested cases here, and instead generate nested
-        // obligations in `confirm_candidate`. However, this doesn't
-        // work because we require handling the recursive cases to
-        // avoid infinite cycles (that is, with recursive types,
-        // sometimes `Foo : Copy` only holds if `Foo : Copy`).
-
         match self.builtin_bound(bound, stack.obligation.self_ty()) {
-            Ok(If(nested)) => {
-                debug!("builtin_bound: bound={} nested={}",
-                       bound.repr(self.tcx()),
-                       nested.repr(self.tcx()));
-                let data = self.vtable_builtin_data(stack.obligation, bound, nested);
-                match self.winnow_selection(Some(stack), VtableBuiltin(data)) {
-                    EvaluatedToOk => { Ok(candidates.vec.push(BuiltinCandidate(bound))) }
-                    EvaluatedToAmbig => { Ok(candidates.ambiguous = true) }
-                    EvaluatedToErr => { Err(Unimplemented) }
-                }
+            Ok(If(_)) => {
+                debug!("builtin_bound: bound={}",
+                       bound.repr(self.tcx()));
+                candidates.vec.push(BuiltinCandidate(bound));
+                Ok(())
             }
             Ok(ParameterBuiltin) => { Ok(()) }
             Ok(AmbiguousBuiltin) => { Ok(candidates.ambiguous = true) }
@@ -1539,8 +1527,11 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                candidate.repr(self.tcx()));
 
         match candidate {
-            // FIXME -- see assemble_builtin_bound_candidates()
-            BuiltinCandidate(_) |
+            BuiltinCandidate(builtin_bound) => {
+                Ok(VtableBuiltin(
+                    try!(self.confirm_builtin_candidate(obligation, builtin_bound))))
+            }
+
             ErrorCandidate => {
                 Ok(VtableBuiltin(VtableBuiltinData { nested: VecPerParamSpace::empty() }))
             }
@@ -1590,8 +1581,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
 
         match try!(self.builtin_bound(bound, obligation.self_ty())) {
             If(nested) => Ok(self.vtable_builtin_data(obligation, bound, nested)),
-            AmbiguousBuiltin |
-            ParameterBuiltin => {
+            AmbiguousBuiltin | ParameterBuiltin => {
                 self.tcx().sess.span_bug(
                     obligation.cause.span,
                     format!("builtin bound for {} was ambig",

src/librustc_trans/trans/base.rs

@@ -965,7 +965,7 @@ pub fn trans_external_path<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>,
 
 pub fn invoke<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
                           llfn: ValueRef,
-                          llargs: Vec<ValueRef> ,
+                          llargs: &[ValueRef],
                           fn_ty: Ty<'tcx>,
                           call_info: Option<NodeInfo>,
                           // FIXME(15064) is_lang_item is a horrible hack, please remove it

src/librustc_trans/trans/callee.rs

@@ -808,7 +808,7 @@ pub fn trans_call_inner<'a, 'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
         // Invoke the actual rust fn and update bcx/llresult.
         let (llret, b) = base::invoke(bcx,
                                       llfn,
-                                      llargs,
+                                      llargs[],
                                       callee_ty,
                                       call_info,
                                       dest.is_none());

src/librustc_trans/trans/glue.rs

@@ -291,7 +291,7 @@ fn trans_struct_drop<'blk, 'tcx>(bcx: Block<'blk, 'tcx>,
         let dtor_ty = ty::mk_ctor_fn(bcx.tcx(),
                                      &[get_drop_glue_type(bcx.ccx(), t)],
                                      ty::mk_nil(bcx.tcx()));
-        let (_, variant_cx) = invoke(variant_cx, dtor_addr, args, dtor_ty, None, false);
+        let (_, variant_cx) = invoke(variant_cx, dtor_addr, args[], dtor_ty, None, false);
 
         variant_cx.fcx.pop_and_trans_custom_cleanup_scope(variant_cx, field_scope);
         variant_cx

src/test/compile-fail/comm-not-freeze-receiver.rs

@@ -0,0 +1,15 @@
+// Copyright 2013 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.
+
+fn test<T: Sync>() {}
+
+fn main() {
+    test::<Receiver<int>>();   //~ ERROR: `core::kinds::Sync` is not implemented
+}

src/test/compile-fail/comm-not-freeze.rs

@@ -11,7 +11,5 @@
 fn test<T: Sync>() {}
 
 fn main() {
-    test::<Sender<int>>();        //~ ERROR: `core::kinds::Sync` is not implemented
-    test::<Receiver<int>>();        //~ ERROR: `core::kinds::Sync` is not implemented
-    test::<Sender<int>>();  //~ ERROR: `core::kinds::Sync` is not implemented
+    test::<Sender<int>>();     //~ ERROR: `core::kinds::Sync` is not implemented
 }

src/test/compile-fail/dst-object-from-unsized-type.rs

@@ -13,18 +13,24 @@
 trait Foo for Sized? {}
 impl Foo for str {}
 
-fn test<Sized? T: Foo>(t: &T) {
+fn test1<Sized? T: Foo>(t: &T) {
     let u: &Foo = t;
     //~^ ERROR `core::kinds::Sized` is not implemented for the type `T`
+}
 
+fn test2<Sized? T: Foo>(t: &T) {
     let v: &Foo = t as &Foo;
     //~^ ERROR `core::kinds::Sized` is not implemented for the type `T`
 }
 
-fn main() {
+fn test3() {
     let _: &[&Foo] = &["hi"];
     //~^ ERROR `core::kinds::Sized` is not implemented for the type `str`
+}
 
+fn test4() {
     let _: &Foo = "hi" as &Foo;
     //~^ ERROR `core::kinds::Sized` is not implemented for the type `str`
 }
+
+fn main() { }

src/test/compile-fail/issue-5883.rs

@@ -18,7 +18,6 @@ fn new_struct(r: A+'static)
     -> Struct { //~^  ERROR the trait `core::kinds::Sized` is not implemented
     //~^ ERROR the trait `core::kinds::Sized` is not implemented
     Struct { r: r }
-    //~^ ERROR the trait `core::kinds::Sized` is not implemented
 }
 
 trait Curve {}

src/test/compile-fail/issue-7013.rs

@@ -32,5 +32,5 @@ struct A {
 
 fn main() {
     let a = A {v: box B{v: None} as Box<Foo+Send>};
-    //~^ ERROR the trait `core::kinds::Send` is not implemented for the type `B`
+    //~^ ERROR the trait `core::kinds::Send` is not implemented
 }