Treat builtin bounds like all other kinds of trait matches. Introduce a simple...

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.

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 @@
 /// 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 @@ pub fn register_obligation(&mut self,
                                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 @@ fn assemble_builtin_bound_candidates<'o>(&mut self,
                                              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 @@ fn confirm_candidate(&mut self,
                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 @@ fn confirm_builtin_candidate(&mut self,
 
         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",