deduplicate trait errors before they are displayed

Because of type inference, duplicate obligations exist and cause duplicate
errors. To avoid this, only display the first error for each (predicate,span).

The inclusion of the span is somewhat bikesheddy, but *is* the more
conservative option (it does not remove some instability, as duplicate
obligations are ignored by `duplicate_set` under some inference conditions).

Fixes #28098
cc #21528 (is it a dupe?)

Author: Ariel Ben-Yehuda

Diff for: src/librustc/middle/infer/mod.rs

@@ -43,7 +43,7 @@ use std::rc::Rc;
 use syntax::ast;
 use syntax::codemap;
 use syntax::codemap::{Span, DUMMY_SP};
-use util::nodemap::{FnvHashMap, NodeMap};
+use util::nodemap::{FnvHashMap, FnvHashSet, NodeMap};
 
 use self::combine::CombineFields;
 use self::region_inference::{RegionVarBindings, RegionSnapshot};
@@ -92,6 +92,10 @@ pub struct InferCtxt<'a, 'tcx: 'a> {
 
     pub fulfillment_cx: RefCell<traits::FulfillmentContext<'tcx>>,
 
+    // the set of predicates on which errors have been reported, to
+    // avoid reporting the same error twice.
+    pub reported_trait_errors: RefCell<FnvHashSet<traits::TraitErrorKey<'tcx>>>,
+
     // This is a temporary field used for toggling on normalization in the inference context,
     // as we move towards the approach described here:
     // https://internals.rust-lang.org/t/flattening-the-contexts-for-fun-and-profit/2293
@@ -374,6 +378,7 @@ pub fn new_infer_ctxt<'a, 'tcx>(tcx: &'a ty::ctxt<'tcx>,
         region_vars: RegionVarBindings::new(tcx),
         parameter_environment: param_env.unwrap_or(tcx.empty_parameter_environment()),
         fulfillment_cx: RefCell::new(traits::FulfillmentContext::new(errors_will_be_reported)),
+        reported_trait_errors: RefCell::new(FnvHashSet()),
         normalize: false,
         err_count_on_creation: tcx.sess.err_count()
     }

Diff for: src/librustc/middle/traits/error_reporting.rs

@@ -33,6 +33,26 @@ use std::fmt;
 use syntax::codemap::Span;
 use syntax::attr::{AttributeMethods, AttrMetaMethods};
 
+#[derive(Debug, PartialEq, Eq, Hash)]
+pub struct TraitErrorKey<'tcx> {
+    is_warning: bool,
+    span: Span,
+    predicate: ty::Predicate<'tcx>
+}
+
+impl<'tcx> TraitErrorKey<'tcx> {
+    fn from_error<'a>(infcx: &InferCtxt<'a, 'tcx>,
+                      e: &FulfillmentError<'tcx>) -> Self {
+        let predicate =
+            infcx.resolve_type_vars_if_possible(&e.obligation.predicate);
+        TraitErrorKey {
+            is_warning: is_warning(&e.obligation),
+            span: e.obligation.cause.span,
+            predicate: infcx.tcx.erase_regions(&predicate)
+        }
+    }
+}
+
 pub fn report_fulfillment_errors<'a, 'tcx>(infcx: &InferCtxt<'a, 'tcx>,
                                            errors: &Vec<FulfillmentError<'tcx>>) {
     for error in errors {
@@ -42,6 +62,13 @@ pub fn report_fulfillment_errors<'a, 'tcx>(infcx: &InferCtxt<'a, 'tcx>,
 
 fn report_fulfillment_error<'a, 'tcx>(infcx: &InferCtxt<'a, 'tcx>,
                                       error: &FulfillmentError<'tcx>) {
+    let error_key = TraitErrorKey::from_error(infcx, error);
+    debug!("report_fulfillment_errors({:?}) - key={:?}",
+           error, error_key);
+    if !infcx.reported_trait_errors.borrow_mut().insert(error_key) {
+        debug!("report_fulfillment_errors: skipping duplicate");
+        return;
+    }
     match error.code {
         FulfillmentErrorCode::CodeSelectionError(ref e) => {
             report_selection_error(infcx, &error.obligation, e);

Diff for: src/librustc/middle/traits/fulfill.rs

@@ -49,6 +49,12 @@ pub struct FulfillmentContext<'tcx> {
     // than the `SelectionCache`: it avoids duplicate errors and
     // permits recursive obligations, which are often generated from
     // traits like `Send` et al.
+    //
+    // Note that because of type inference, a predicate can still
+    // occur twice in the predicates list, for example when 2
+    // initially-distinct type variables are unified after being
+    // inserted. Deduplicating the predicate set on selection had a
+    // significant performance cost the last time I checked.
     duplicate_set: FulfilledPredicates<'tcx>,
 
     // A list of all obligations that have been registered with this

Diff for: src/librustc/middle/traits/mod.rs

@@ -21,10 +21,12 @@ use middle::subst;
 use middle::ty::{self, HasTypeFlags, Ty};
 use middle::ty::fold::TypeFoldable;
 use middle::infer::{self, fixup_err_to_string, InferCtxt};
+
 use std::rc::Rc;
 use syntax::ast;
 use syntax::codemap::{Span, DUMMY_SP};
 
+pub use self::error_reporting::TraitErrorKey;
 pub use self::error_reporting::report_fulfillment_errors;
 pub use self::error_reporting::report_overflow_error;
 pub use self::error_reporting::report_selection_error;

Diff for: src/test/compile-fail/associated-types-ICE-when-projecting-out-of-err.rs

@@ -32,5 +32,4 @@ fn ice<A>(a: A) {
     let r = loop {};
     r = r + a;
     //~^ ERROR not implemented
-    //~| ERROR not implemented
 }

Diff for: src/test/compile-fail/associated-types-path-2.rs

@@ -39,7 +39,6 @@ pub fn f1_int_uint() {
 pub fn f1_uint_uint() {
     f1(2u32, 4u32);
     //~^ ERROR the trait `Foo` is not implemented
-    //~| ERROR the trait `Foo` is not implemented
 }
 
 pub fn f1_uint_int() {

Diff for: src/test/compile-fail/coerce-unsafe-to-closure.rs

@@ -11,5 +11,4 @@
 fn main() {
     let x: Option<&[u8]> = Some("foo").map(std::mem::transmute);
     //~^ ERROR E0277
-    //~| ERROR E0277
 }

Diff for: src/test/compile-fail/const-eval-overflow-4b.rs

@@ -23,7 +23,6 @@ const A_I8_T
     : [u32; (i8::MAX as i8 + 1u8) as usize]
     //~^ ERROR mismatched types
     //~| the trait `core::ops::Add<u8>` is not implemented for the type `i8`
-    //~| the trait `core::ops::Add<u8>` is not implemented for the type `i8`
     = [0; (i8::MAX as usize) + 1];
 
 fn main() {
@@ -33,4 +32,3 @@ fn main() {
 fn foo<T:fmt::Debug>(x: T) {
     println!("{:?}", x);
 }
-

Diff for: src/test/compile-fail/fn-variance-1.rs

@@ -20,10 +20,8 @@ fn main() {
     apply(&3, takes_imm);
     apply(&3, takes_mut);
     //~^ ERROR (values differ in mutability)
-    //~| ERROR (values differ in mutability)
 
     apply(&mut 3, takes_mut);
     apply(&mut 3, takes_imm);
     //~^ ERROR (values differ in mutability)
-    //~| ERROR (values differ in mutability)
 }

Diff for: src/test/compile-fail/for-loop-bogosity.rs

@@ -25,9 +25,6 @@ pub fn main() {
         y: 2,
     };
     for x in bogus { //~ ERROR `core::iter::Iterator` is not implemented for the type `MyStruct`
-    //~^ ERROR
-    //~^^ ERROR
-    // FIXME(#21528) not fulfilled obligation error should be reported once, not thrice
         drop(x);
     }
 }

Diff for: src/test/compile-fail/indexing-requires-a-uint.rs

@@ -14,7 +14,6 @@
 fn main() {
     fn bar<T>(_: T) {}
     [0][0u8]; //~ ERROR: the trait `core::ops::Index<u8>` is not implemented
-    //~^ ERROR: the trait `core::ops::Index<u8>` is not implemented
 
     [0][0]; // should infer to be a usize
 

Diff for: src/test/compile-fail/integral-indexing.rs

@@ -14,21 +14,13 @@ pub fn main() {
     v[3_usize];
     v[3];
     v[3u8];  //~ERROR the trait `core::ops::Index<u8>` is not implemented
-    //~^ ERROR the trait `core::ops::Index<u8>` is not implemented
     v[3i8];  //~ERROR the trait `core::ops::Index<i8>` is not implemented
-    //~^ ERROR the trait `core::ops::Index<i8>` is not implemented
     v[3u32]; //~ERROR the trait `core::ops::Index<u32>` is not implemented
-    //~^ ERROR the trait `core::ops::Index<u32>` is not implemented
     v[3i32]; //~ERROR the trait `core::ops::Index<i32>` is not implemented
-    //~^ ERROR the trait `core::ops::Index<i32>` is not implemented
     s.as_bytes()[3_usize];
     s.as_bytes()[3];
     s.as_bytes()[3u8];  //~ERROR the trait `core::ops::Index<u8>` is not implemented
-    //~^ ERROR the trait `core::ops::Index<u8>` is not implemented
     s.as_bytes()[3i8];  //~ERROR the trait `core::ops::Index<i8>` is not implemented
-    //~^ ERROR the trait `core::ops::Index<i8>` is not implemented
     s.as_bytes()[3u32]; //~ERROR the trait `core::ops::Index<u32>` is not implemented
-    //~^ ERROR the trait `core::ops::Index<u32>` is not implemented
     s.as_bytes()[3i32]; //~ERROR the trait `core::ops::Index<i32>` is not implemented
-    //~^ ERROR the trait `core::ops::Index<i32>` is not implemented
 }

Diff for: src/test/compile-fail/issue-11771.rs

@@ -12,12 +12,10 @@ fn main() {
     let x = ();
     1 +
     x //~^ ERROR E0277
-      //~| ERROR E0277
     ;
 
     let x: () = ();
     1 +
     x //~^ ERROR E0277
-      //~| ERROR E0277
     ;
 }

Diff for: src/test/compile-fail/issue-13352.rs

@@ -18,5 +18,4 @@ fn main() {
     });
     2_usize + (loop {});
     //~^ ERROR E0277
-    //~| ERROR E0277
 }

Diff for: src/test/compile-fail/issue-14084.rs

@@ -14,5 +14,4 @@
 fn main() {
     in () { 0 };
     //~^ ERROR: the trait `core::ops::Placer<_>` is not implemented
-    //~| ERROR: the trait `core::ops::Placer<_>` is not implemented
 }

Diff for: src/test/compile-fail/issue-20605.rs

@@ -11,9 +11,6 @@
 fn changer<'a>(mut things: Box<Iterator<Item=&'a mut u8>>) {
     for item in *things { *item = 0 }
 //~^ ERROR the trait `core::marker::Sized` is not implemented for the type `core::iter::Iterator
-//~^^ ERROR
-//~^^^ ERROR
-// FIXME(#21528) error should be reported once, not thrice
 }
 
 fn main() {}

Diff for: src/test/compile-fail/issue-2149.rs

@@ -17,7 +17,6 @@ impl<A> vec_monad<A> for Vec<A> {
         let mut r = panic!();
         for elt in self { r = r + f(*elt); }
         //~^ ERROR E0277
-        //~| ERROR E0277
    }
 }
 fn main() {

Diff for: src/test/compile-fail/issue-22645.rs

@@ -23,7 +23,5 @@ impl<RHS: Scalar> Add <RHS> for Bob {
 fn main() {
   let b = Bob + 3.5;
   b + 3 //~ ERROR: is not implemented
-  //~^ ERROR: is not implemented
-  //~^^ ERROR: is not implemented
-  //~^^^ ERROR: mismatched types
+  //~^ ERROR: mismatched types
 }

Diff for: src/test/compile-fail/issue-24352.rs

@@ -11,5 +11,4 @@
 fn main() {
     1.0f64 - 1.0;
     1.0f64 - 1 //~ ERROR: is not implemented
-    //~^ ERROR: is not implemented
 }

Diff for: src/test/compile-fail/issue-28098.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.
+
+fn main() {
+    let _ = Iterator::next(&mut ());
+    //~^ ERROR the trait `core::iter::Iterator` is not implemented
+
+    for _ in false {}
+    //~^ ERROR the trait `core::iter::Iterator` is not implemented
+
+    let _ = Iterator::next(&mut ());
+    //~^ ERROR the trait `core::iter::Iterator` is not implemented
+
+    other()
+}
+
+pub fn other() {
+    // check errors are still reported globally
+
+    let _ = Iterator::next(&mut ());
+    //~^ ERROR the trait `core::iter::Iterator` is not implemented
+
+    let _ = Iterator::next(&mut ());
+    //~^ ERROR the trait `core::iter::Iterator` is not implemented
+
+    for _ in false {}
+    //~^ ERROR the trait `core::iter::Iterator` is not implemented
+}

Diff for: src/test/compile-fail/shift-various-bad-types.rs

@@ -18,15 +18,12 @@ struct Panolpy {
 fn foo(p: &Panolpy) {
     22 >> p.char;
     //~^ ERROR E0277
-    //~| ERROR E0277
 
     22 >> p.str;
     //~^ ERROR E0277
-    //~| ERROR E0277
 
     22 >> p;
     //~^ ERROR E0277
-    //~| ERROR E0277
 
     let x;
     22 >> x; // ambiguity error winds up being suppressed

Diff for: src/test/compile-fail/str-idx.rs

@@ -11,5 +11,4 @@
 pub fn main() {
     let s: &str = "hello";
     let c: u8 = s[4]; //~ ERROR the trait `core::ops::Index<_>` is not implemented
-    //~^ ERROR the trait `core::ops::Index<_>` is not implemented
 }

Diff for: src/test/compile-fail/str-mut-idx.rs

@@ -17,7 +17,6 @@ fn mutate(s: &mut str) {
     s[1usize] = bot();
     //~^ ERROR `core::ops::Index<usize>` is not implemented for the type `str`
     //~| ERROR `core::ops::IndexMut<usize>` is not implemented for the type `str`
-    //~| ERROR `core::ops::Index<usize>` is not implemented for the type `str`
 }
 
 pub fn main() {}

Diff for: src/test/compile-fail/unboxed-closures-unsafe-extern-fn.rs

@@ -35,7 +35,6 @@ fn b() {
 fn c() {
     let z = call_it_once(square, 22);
     //~^ ERROR not implemented
-    //~| ERROR not implemented
 }
 
 fn main() { }

Diff for: src/test/compile-fail/unboxed-closures-wrong-abi.rs

@@ -35,7 +35,6 @@ fn b() {
 fn c() {
     let z = call_it_once(square, 22);
     //~^ ERROR not implemented
-    //~| ERROR not implemented
 }
 
 fn main() { }

Diff for: src/test/compile-fail/unboxed-closures-wrong-arg-type-extern-fn.rs

@@ -36,7 +36,6 @@ fn b() {
 fn c() {
     let z = call_it_once(square, 22);
     //~^ ERROR not implemented
-    //~| ERROR not implemented
 }
 
 fn main() { }