make a custom error for overlap with negative impls

Author: nikomatsakis

src/librustc_error_codes/error_codes.rs

@@ -427,6 +427,7 @@ E0747: include_str!("./error_codes/E0747.md"),
 E0748: include_str!("./error_codes/E0748.md"),
 E0749: include_str!("./error_codes/E0749.md"),
 E0750: include_str!("./error_codes/E0750.md"),
+E0751: include_str!("./error_codes/E0751.md"),
 ;
 //  E0006, // merged with E0005
 //  E0008, // cannot bind by-move into a pattern guard

src/librustc_error_codes/error_codes/E0751.md

@@ -0,0 +1,12 @@
+There are both a positive and negative trait implementation for the same type.
+
+Erroneous code example:
+
+```compile_fail,E0748
+trait MyTrait {}
+impl MyTrait for i32 { }
+impl !MyTrait for i32 { }
+```
+
+Negative implementations are a promise that the trait will never be
+implemented for the given types.

src/librustc_trait_selection/traits/specialize/mod.rs

@@ -324,86 +324,7 @@ pub(super) fn specialization_graph_provider(
             };
 
             if let Some(overlap) = overlap {
-                let impl_span =
-                    tcx.sess.source_map().guess_head_span(tcx.span_of_impl(impl_def_id).unwrap());
-
-                // Work to be done after we've built the DiagnosticBuilder. We have to define it
-                // now because the struct_lint methods don't return back the DiagnosticBuilder
-                // that's passed in.
-                let decorate = |err: LintDiagnosticBuilder<'_>| {
-                    let msg = format!(
-                        "conflicting implementations of trait `{}`{}:{}",
-                        overlap.trait_desc,
-                        overlap
-                            .self_desc
-                            .clone()
-                            .map_or(String::new(), |ty| { format!(" for type `{}`", ty) }),
-                        match used_to_be_allowed {
-                            Some(FutureCompatOverlapErrorKind::Issue33140) => " (E0119)",
-                            _ => "",
-                        }
-                    );
-                    let mut err = err.build(&msg);
-                    match tcx.span_of_impl(overlap.with_impl) {
-                        Ok(span) => {
-                            err.span_label(
-                                tcx.sess.source_map().guess_head_span(span),
-                                "first implementation here".to_string(),
-                            );
-
-                            err.span_label(
-                                impl_span,
-                                format!(
-                                    "conflicting implementation{}",
-                                    overlap
-                                        .self_desc
-                                        .map_or(String::new(), |ty| format!(" for `{}`", ty))
-                                ),
-                            );
-                        }
-                        Err(cname) => {
-                            let msg = match to_pretty_impl_header(tcx, overlap.with_impl) {
-                                Some(s) => format!(
-                                    "conflicting implementation in crate `{}`:\n- {}",
-                                    cname, s
-                                ),
-                                None => format!("conflicting implementation in crate `{}`", cname),
-                            };
-                            err.note(&msg);
-                        }
-                    }
-
-                    for cause in &overlap.intercrate_ambiguity_causes {
-                        cause.add_intercrate_ambiguity_hint(&mut err);
-                    }
-
-                    if overlap.involves_placeholder {
-                        coherence::add_placeholder_note(&mut err);
-                    }
-                    err.emit()
-                };
-
-                match used_to_be_allowed {
-                    None => {
-                        sg.has_errored = true;
-                        let err = struct_span_err!(tcx.sess, impl_span, E0119, "");
-                        decorate(LintDiagnosticBuilder::new(err));
-                    }
-                    Some(kind) => {
-                        let lint = match kind {
-                            FutureCompatOverlapErrorKind::Issue33140 => {
-                                ORDER_DEPENDENT_TRAIT_OBJECTS
-                            }
-                            FutureCompatOverlapErrorKind::LeakCheck => COHERENCE_LEAK_CHECK,
-                        };
-                        tcx.struct_span_lint_hir(
-                            lint,
-                            tcx.hir().as_local_hir_id(impl_def_id).unwrap(),
-                            impl_span,
-                            decorate,
-                        )
-                    }
-                };
+                report_overlap_conflict(tcx, overlap, impl_def_id, used_to_be_allowed, &mut sg);
             }
         } else {
             let parent = tcx.impl_parent(impl_def_id).unwrap_or(trait_id);
@@ -414,6 +335,170 @@ pub(super) fn specialization_graph_provider(
     tcx.arena.alloc(sg)
 }
 
+fn report_overlap_conflict(
+    tcx: TyCtxt<'_>,
+    overlap: OverlapError,
+    impl_def_id: DefId,
+    used_to_be_allowed: Option<FutureCompatOverlapErrorKind>,
+    sg: &mut specialization_graph::Graph,
+) {
+    let impl_polarity = tcx.impl_polarity(impl_def_id);
+    let other_polarity = tcx.impl_polarity(overlap.with_impl);
+    match (impl_polarity, other_polarity) {
+        (ty::ImplPolarity::Negative, ty::ImplPolarity::Positive) => {
+            report_negative_positive_conflict(
+                tcx,
+                &overlap,
+                impl_def_id,
+                impl_def_id,
+                overlap.with_impl,
+                sg,
+            );
+        }
+
+        (ty::ImplPolarity::Positive, ty::ImplPolarity::Negative) => {
+            report_negative_positive_conflict(
+                tcx,
+                &overlap,
+                impl_def_id,
+                overlap.with_impl,
+                impl_def_id,
+                sg,
+            );
+        }
+
+        _ => {
+            report_conflicting_impls(tcx, overlap, impl_def_id, used_to_be_allowed, sg);
+        }
+    }
+}
+
+fn report_negative_positive_conflict(
+    tcx: TyCtxt<'_>,
+    overlap: &OverlapError,
+    local_impl_def_id: DefId,
+    negative_impl_def_id: DefId,
+    positive_impl_def_id: DefId,
+    sg: &mut specialization_graph::Graph,
+) {
+    let impl_span =
+        tcx.sess.source_map().guess_head_span(tcx.span_of_impl(local_impl_def_id).unwrap());
+
+    let mut err = struct_span_err!(
+        tcx.sess,
+        impl_span,
+        E0748,
+        "found both positive and negative implementation of trait `{}`{}:",
+        overlap.trait_desc,
+        overlap.self_desc.clone().map_or(String::new(), |ty| format!(" for type `{}`", ty))
+    );
+
+    match tcx.span_of_impl(negative_impl_def_id) {
+        Ok(span) => {
+            err.span_label(
+                tcx.sess.source_map().guess_head_span(span),
+                "negative implementation here".to_string(),
+            );
+        }
+        Err(cname) => {
+            err.note(&format!("negative implementation in crate `{}`", cname));
+        }
+    }
+
+    match tcx.span_of_impl(positive_impl_def_id) {
+        Ok(span) => {
+            err.span_label(
+                tcx.sess.source_map().guess_head_span(span),
+                "positive implementation here".to_string(),
+            );
+        }
+        Err(cname) => {
+            err.note(&format!("positive implementation in crate `{}`", cname));
+        }
+    }
+
+    sg.has_errored = true;
+    err.emit();
+}
+
+fn report_conflicting_impls(
+    tcx: TyCtxt<'_>,
+    overlap: OverlapError,
+    impl_def_id: DefId,
+    used_to_be_allowed: Option<FutureCompatOverlapErrorKind>,
+    sg: &mut specialization_graph::Graph,
+) {
+    let impl_span = tcx.sess.source_map().guess_head_span(tcx.span_of_impl(impl_def_id).unwrap());
+
+    // Work to be done after we've built the DiagnosticBuilder. We have to define it
+    // now because the struct_lint methods don't return back the DiagnosticBuilder
+    // that's passed in.
+    let decorate = |err: LintDiagnosticBuilder<'_>| {
+        let msg = format!(
+            "conflicting implementations of trait `{}`{}:{}",
+            overlap.trait_desc,
+            overlap.self_desc.clone().map_or(String::new(), |ty| { format!(" for type `{}`", ty) }),
+            match used_to_be_allowed {
+                Some(FutureCompatOverlapErrorKind::Issue33140) => " (E0119)",
+                _ => "",
+            }
+        );
+        let mut err = err.build(&msg);
+        match tcx.span_of_impl(overlap.with_impl) {
+            Ok(span) => {
+                err.span_label(
+                    tcx.sess.source_map().guess_head_span(span),
+                    "first implementation here".to_string(),
+                );
+
+                err.span_label(
+                    impl_span,
+                    format!(
+                        "conflicting implementation{}",
+                        overlap.self_desc.map_or(String::new(), |ty| format!(" for `{}`", ty))
+                    ),
+                );
+            }
+            Err(cname) => {
+                let msg = match to_pretty_impl_header(tcx, overlap.with_impl) {
+                    Some(s) => format!("conflicting implementation in crate `{}`:\n- {}", cname, s),
+                    None => format!("conflicting implementation in crate `{}`", cname),
+                };
+                err.note(&msg);
+            }
+        }
+
+        for cause in &overlap.intercrate_ambiguity_causes {
+            cause.add_intercrate_ambiguity_hint(&mut err);
+        }
+
+        if overlap.involves_placeholder {
+            coherence::add_placeholder_note(&mut err);
+        }
+        err.emit()
+    };
+
+    match used_to_be_allowed {
+        None => {
+            sg.has_errored = true;
+            let err = struct_span_err!(tcx.sess, impl_span, E0119, "");
+            decorate(LintDiagnosticBuilder::new(err));
+        }
+        Some(kind) => {
+            let lint = match kind {
+                FutureCompatOverlapErrorKind::Issue33140 => ORDER_DEPENDENT_TRAIT_OBJECTS,
+                FutureCompatOverlapErrorKind::LeakCheck => COHERENCE_LEAK_CHECK,
+            };
+            tcx.struct_span_lint_hir(
+                lint,
+                tcx.hir().as_local_hir_id(impl_def_id).unwrap(),
+                impl_span,
+                decorate,
+            )
+        }
+    };
+}
+
 /// Recovers the "impl X for Y" signature from `impl_def_id` and returns it as a
 /// string.
 fn to_pretty_impl_header(tcx: TyCtxt<'_>, impl_def_id: DefId) -> Option<String> {

src/test/ui/coherence/coherence-conflicting-negative-trait-impl.rs

@@ -6,13 +6,11 @@ trait MyTrait {}
 
 struct TestType<T>(::std::marker::PhantomData<T>);
 
-unsafe impl<T: MyTrait+'static> Send for TestType<T> {}
+unsafe impl<T: MyTrait + 'static> Send for TestType<T> {}
 
-impl<T: MyTrait> !Send for TestType<T> {}
-//~^ ERROR conflicting implementations
+impl<T: MyTrait> !Send for TestType<T> {} //~ ERROR found both positive and negative implementation
 
-unsafe impl<T:'static> Send for TestType<T> {}
-//~^ ERROR conflicting implementations
+unsafe impl<T: 'static> Send for TestType<T> {} //~ ERROR conflicting implementations
 
 impl !Send for TestType<i32> {}
 

src/test/ui/coherence/coherence-conflicting-negative-trait-impl.stderr

@@ -1,21 +1,22 @@
-error[E0119]: conflicting implementations of trait `std::marker::Send` for type `TestType<_>`:
+error[E0748]: found both positive and negative implementation of trait `std::marker::Send` for type `TestType<_>`:
   --> $DIR/coherence-conflicting-negative-trait-impl.rs:11:1
    |
-LL | unsafe impl<T: MyTrait+'static> Send for TestType<T> {}
-   | ---------------------------------------------------- first implementation here
+LL | unsafe impl<T: MyTrait + 'static> Send for TestType<T> {}
+   | ------------------------------------------------------ positive implementation here
 LL | 
 LL | impl<T: MyTrait> !Send for TestType<T> {}
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `TestType<_>`
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ negative implementation here
 
 error[E0119]: conflicting implementations of trait `std::marker::Send` for type `TestType<_>`:
-  --> $DIR/coherence-conflicting-negative-trait-impl.rs:14:1
+  --> $DIR/coherence-conflicting-negative-trait-impl.rs:13:1
    |
-LL | unsafe impl<T: MyTrait+'static> Send for TestType<T> {}
-   | ---------------------------------------------------- first implementation here
+LL | unsafe impl<T: MyTrait + 'static> Send for TestType<T> {}
+   | ------------------------------------------------------ first implementation here
 ...
-LL | unsafe impl<T:'static> Send for TestType<T> {}
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `TestType<_>`
+LL | unsafe impl<T: 'static> Send for TestType<T> {}
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ conflicting implementation for `TestType<_>`
 
 error: aborting due to 2 previous errors
 
-For more information about this error, try `rustc --explain E0119`.
+Some errors have detailed explanations: E0119, E0748.
+For more information about an error, try `rustc --explain E0119`.