Check for negative impls for `Send` and `Sync`

5 files changed, 87 insertions, 31 deletions

diff --git a/src/librustc/middle/traits/select.rs b/src/librustc/middle/traits/select.rs
index 51a793f1de8..2e2d64e5636 100644
--- a/src/librustc/middle/traits/select.rs
+++ b/src/librustc/middle/traits/select.rs
@@ -611,6 +611,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
             return Ok(None);
         }
 
+
         // If there are *NO* candidates, that there are no impls --
         // that we know of, anyway. Note that in the case where there
         // are unbound type variables within the obligation, it might
@@ -626,6 +627,17 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
 
         // Just one candidate left.
         let candidate = candidates.pop().unwrap();
+
+        match candidate {
+            ImplCandidate(def_id) => {
+                match ty::trait_impl_polarity(self.tcx(), def_id) {
+                    Some(ast::ImplPolarity::Negative) => return Err(Unimplemented),
+                    _ => {}
+                }
+            }
+            _ => {}
+        }
+
         Ok(Some(candidate))
     }
 
@@ -714,7 +726,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                 debug!("obligation self ty is {}",
                        obligation.predicate.0.self_ty().repr(self.tcx()));
 
-                try!(self.assemble_candidates_from_impls(obligation, &mut candidates.vec));
+                try!(self.assemble_candidates_from_impls(obligation, &mut candidates));
 
                 try!(self.assemble_builtin_bound_candidates(ty::BoundCopy,
                                                             stack,
@@ -722,7 +734,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
             }
             Some(bound @ ty::BoundSend) |
             Some(bound @ ty::BoundSync) => {
-                try!(self.assemble_candidates_from_impls(obligation, &mut candidates.vec));
+                try!(self.assemble_candidates_from_impls(obligation, &mut candidates));
 
                 // No explicit impls were declared for this type, consider the fallback rules.
                 if candidates.vec.is_empty() && !candidates.ambiguous {
@@ -741,7 +753,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                 // (And unboxed candidates only apply to the Fn/FnMut/etc traits.)
                 try!(self.assemble_unboxed_closure_candidates(obligation, &mut candidates));
                 try!(self.assemble_fn_pointer_candidates(obligation, &mut candidates));
-                try!(self.assemble_candidates_from_impls(obligation, &mut candidates.vec));
+                try!(self.assemble_candidates_from_impls(obligation, &mut candidates));
                 self.assemble_candidates_from_object_ty(obligation, &mut candidates);
             }
         }
@@ -1013,9 +1025,12 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
     /// Search for impls that might apply to `obligation`.
     fn assemble_candidates_from_impls(&mut self,
                                       obligation: &TraitObligation<'tcx>,
-                                      candidate_vec: &mut Vec<SelectionCandidate<'tcx>>)
+                                      candidates: &mut SelectionCandidateSet<'tcx>)
                                       -> Result<(), SelectionError<'tcx>>
     {
+        let self_ty = self.infcx.shallow_resolve(obligation.self_ty());
+        debug!("assemble_candidates_from_impls(self_ty={})", self_ty.repr(self.tcx()));
+
         let all_impls = self.all_impls(obligation.predicate.def_id());
         for &impl_def_id in all_impls.iter() {
             self.infcx.probe(|snapshot| {
@@ -1024,7 +1039,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
                 match self.match_impl(impl_def_id, obligation, snapshot,
                                       &skol_map, skol_obligation_trait_pred.trait_ref.clone()) {
                     Ok(_) => {
-                        candidate_vec.push(ImplCandidate(impl_def_id));
+                        candidates.vec.push(ImplCandidate(impl_def_id));
                     }
                     Err(()) => { }
                 }
@@ -2214,12 +2229,19 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
 
     /// Returns set of all impls for a given trait.
     fn all_impls(&self, trait_def_id: ast::DefId) -> Vec<ast::DefId> {
-        ty::populate_implementations_for_trait_if_necessary(self.tcx(),
-                                                            trait_def_id);
-        match self.tcx().trait_impls.borrow().get(&trait_def_id) {
+        ty::populate_implementations_for_trait_if_necessary(self.tcx(), trait_def_id);
+
+        let mut trait_impls = match self.tcx().trait_impls.borrow().get(&trait_def_id) {
             None => Vec::new(),
             Some(impls) => impls.borrow().clone()
-        }
+        };
+
+        match self.tcx().trait_negative_impls.borrow().get(&trait_def_id) {
+            None => {},
+            Some(impls) => trait_impls.push_all(impls.borrow().as_slice()),
+        };
+
+        trait_impls
     }
 
     fn impl_obligations(&mut self,
diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs
index e4c333a0e1e..7e38321049e 100644
--- a/src/librustc_typeck/check/mod.rs
+++ b/src/librustc_typeck/check/mod.rs
@@ -1597,7 +1597,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
     {
         debug!("register_predicate({})",
                obligation.repr(self.tcx()));
-
         self.inh.fulfillment_cx
             .borrow_mut()
             .register_predicate_obligation(self.infcx(), obligation);
diff --git a/src/librustc_typeck/check/wf.rs b/src/librustc_typeck/check/wf.rs
index 89de1ea80fc..baa4850ce4a 100644
--- a/src/librustc_typeck/check/wf.rs
+++ b/src/librustc_typeck/check/wf.rs
@@ -56,7 +56,24 @@ impl<'ccx, 'tcx> CheckTypeWellFormedVisitor<'ccx, 'tcx> {
                ty::item_path_str(ccx.tcx, local_def(item.id)));
 
         match item.node {
-            ast::ItemImpl(..) => {
+            /// Right now we check that every default trait implementation
+            /// has an implementation of itself. Basically, a case like:
+            ///
+            /// `impl Trait for T {}`
+            ///
+            /// has a requirement of `T: Trait` which was required for default
+            /// method implementations. Although this could be improved now that
+            /// there's a better infrastructure in place for this, it's being left
+            /// for a follow-up work.
+            ///
+            /// Since there's such a requirement, we need to check *just* positive
+            /// implementations, otherwise things like:
+            ///
+            /// impl !Send for T {}
+            ///
+            /// won't be allowed unless there's an *explicit* implementation of `Send`
+            /// for `T`
+            ast::ItemImpl(_, ast::ImplPolarity::Positive, _, _, _, _) => {
                 self.check_impl(item);
             }
             ast::ItemFn(..) => {
diff --git a/src/test/compile-fail/coherence-trait-polarity-mising-default-implementation.rs b/src/test/compile-fail/coherence-trait-polarity-mising-default-implementation.rs
deleted file mode 100644
index d882603655b..00000000000
--- a/src/test/compile-fail/coherence-trait-polarity-mising-default-implementation.rs
+++ /dev/null
@@ -1,20 +0,0 @@
-// 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.
-
-#![feature(optin_builtin_traits)]
-
-struct TestType;
-
-trait TestTrait {}
-
-impl !TestTrait for TestType {}
-//~^ the trait `TestTrait` is not implemented for the type `TestType`
-
-fn main() {}
diff --git a/src/test/compile-fail/traits-negative-impls.rs b/src/test/compile-fail/traits-negative-impls.rs
new file mode 100644
index 00000000000..a7d1d796801
--- /dev/null
+++ b/src/test/compile-fail/traits-negative-impls.rs
@@ -0,0 +1,38 @@
+// 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.
+
+#![feature(optin_builtin_traits)]
+
+use std::marker::Send;
+
+struct Outer<T: Send>(T);
+
+struct TestType;
+impl !Send for TestType {}
+
+struct Outer2<T>(T);
+
+unsafe impl<T: Send> Sync for Outer2<T> {}
+
+fn is_send<T: Send>(_: T) {}
+fn is_sync<T: Sync>(_: T) {}
+
+fn main() {
+    Outer(TestType);
+    //~^ ERROR the trait `core::marker::Send` is not implemented for the type `TestType`
+
+    is_send(TestType);
+    //~^ ERROR the trait `core::marker::Send` is not implemented for the type `TestType`
+
+    // This will complain about a missing Send impl because `Sync` is implement *just*
+    // for T that are `Send`. Look at #20366 and #19950
+    is_sync(Outer2(TestType));
+    //~^ ERROR the trait `core::marker::Send` is not implemented for the type `TestType`
+}