RFC 2027: Improve Diagnostics

bovinebuddha · bovinebuddha · commit ed822d264587 · 2019-01-27T13:54:33.000+01:00
diff --git a/src/librustc/infer/error_reporting/mod.rs b/src/librustc/infer/error_reporting/mod.rs
@@ -977,10 +977,17 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
 
         let span = cause.span(&self.tcx);
 
-        diag.span_label(span, terr.to_string());
-        if let Some((sp, msg)) = secondary_span {
-            diag.span_label(sp, msg);
-        }
+        // Ignore msg for object safe coercion
+        // since E0038 message will be printed
+        match terr {
+            TypeError::ObjectUnsafeCoercion(_) => {}
+            _ => {
+                diag.span_label(span, terr.to_string());
+                if let Some((sp, msg)) = secondary_span {
+                    diag.span_label(sp, msg);
+                }
+            }
+        };
 
         if let Some((expected, found)) = expected_found {
             match (terr, is_simple_error, expected == found) {
@@ -993,6 +1000,9 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
                         &format!(" ({})", values.found.sort_string(self.tcx)),
                     );
                 }
+                (TypeError::ObjectUnsafeCoercion(_), ..) => {
+                    diag.note_unsuccessfull_coercion(found, expected);
+                }
                 (_, false, _) => {
                     if let Some(exp_found) = exp_found {
                         let (def_id, ret_ty) = match exp_found.found.sty {
@@ -1112,6 +1122,11 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
         let span = trace.cause.span(&self.tcx);
         let failure_code = trace.cause.as_failure_code(terr);
         let mut diag = match failure_code {
+            FailureCode::Error0038(did) => {
+                let violations = self.tcx.global_tcx()
+                    .object_safety_violations(did);
+                self.tcx.report_object_safety_error(span, did, violations)
+            }
             FailureCode::Error0317(failure_str) => {
                 struct_span_err!(self.tcx.sess, span, E0317, "{}", failure_str)
             }
@@ -1470,6 +1485,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> {
 }
 
 enum FailureCode {
+    Error0038(DefId),
     Error0317(&'static str),
     Error0580(&'static str),
     Error0308(&'static str),
@@ -1503,6 +1519,7 @@ impl<'tcx> ObligationCause<'tcx> {
                 TypeError::CyclicTy(ty) if ty.is_closure() || ty.is_generator() => {
                     Error0644("closure/generator type that references itself")
                 }
+                TypeError::ObjectUnsafeCoercion(did) => Error0038(did.clone()),
                 _ => Error0308("mismatched types"),
             },
         }
diff --git a/src/librustc/ty/error.rs b/src/librustc/ty/error.rs
@@ -42,6 +42,7 @@ pub enum TypeError<'tcx> {
     ProjectionMismatched(ExpectedFound<DefId>),
     ProjectionBoundsLength(ExpectedFound<usize>),
     ExistentialMismatch(ExpectedFound<&'tcx ty::List<ty::ExistentialPredicate<'tcx>>>),
+    ObjectUnsafeCoercion(DefId),
 }
 
 #[derive(Clone, RustcEncodable, RustcDecodable, PartialEq, Eq, Hash, Debug, Copy)]
@@ -144,6 +145,7 @@ impl<'tcx> fmt::Display for TypeError<'tcx> {
                 report_maybe_different(f, &format!("trait `{}`", values.expected),
                                        &format!("trait `{}`", values.found))
             }
+            ObjectUnsafeCoercion(_) => write!(f, "coercion to object-unsafe trait object"),
         }
     }
 }
diff --git a/src/librustc/ty/structural_impls.rs b/src/librustc/ty/structural_impls.rs
@@ -443,7 +443,8 @@ impl<'a, 'tcx> Lift<'tcx> for ty::error::TypeError<'a> {
             ProjectionMismatched(x) => ProjectionMismatched(x),
             ProjectionBoundsLength(x) => ProjectionBoundsLength(x),
             Sorts(ref x) => return tcx.lift(x).map(Sorts),
-            ExistentialMismatch(ref x) => return tcx.lift(x).map(ExistentialMismatch)
+            ExistentialMismatch(ref x) => return tcx.lift(x).map(ExistentialMismatch),
+            ObjectUnsafeCoercion(defid) => ObjectUnsafeCoercion(defid),
         })
     }
 }
@@ -1031,6 +1032,7 @@ EnumTypeFoldableImpl! {
         (ty::error::TypeError::ProjectionBoundsLength)(x),
         (ty::error::TypeError::Sorts)(x),
         (ty::error::TypeError::ExistentialMismatch)(x),
+        (ty::error::TypeError::ObjectUnsafeCoercion)(x),
     }
 }
 
diff --git a/src/librustc_errors/diagnostic.rs b/src/librustc_errors/diagnostic.rs
@@ -149,6 +149,32 @@ impl Diagnostic {
         self.note_expected_found_extra(label, expected, found, &"", &"")
     }
 
+    pub fn note_unsuccessfull_coercion(&mut self,
+                                       expected: DiagnosticStyledString,
+                                       found: DiagnosticStyledString)
+                                       -> &mut Self
+    {
+        let mut msg: Vec<_> =
+            vec![(format!("required when trying to coerce from type `"),
+                  Style::NoStyle)];
+        msg.extend(expected.0.iter()
+                   .map(|x| match *x {
+                       StringPart::Normal(ref s) => (s.to_owned(), Style::NoStyle),
+                       StringPart::Highlighted(ref s) => (s.to_owned(), Style::Highlight),
+                   }));
+        msg.push((format!("` to type '"), Style::NoStyle));
+        msg.extend(found.0.iter()
+                   .map(|x| match *x {
+                       StringPart::Normal(ref s) => (s.to_owned(), Style::NoStyle),
+                       StringPart::Highlighted(ref s) => (s.to_owned(), Style::Highlight),
+                   }));
+        msg.push((format!("`"), Style::NoStyle));
+
+        // For now, just attach these as notes
+        self.highlighted_note(msg);
+        self
+    }
+
     pub fn note_expected_found_extra(&mut self,
                                      label: &dyn fmt::Display,
                                      expected: DiagnosticStyledString,
diff --git a/src/librustc_errors/diagnostic_builder.rs b/src/librustc_errors/diagnostic_builder.rs
@@ -174,6 +174,11 @@ impl<'a> DiagnosticBuilder<'a> {
                                               found_extra: &dyn fmt::Display,
                                               ) -> &mut Self);
 
+    forward!(pub fn note_unsuccessfull_coercion(&mut self,
+                                                expected: DiagnosticStyledString,
+                                                found: DiagnosticStyledString,
+                                                ) -> &mut Self);
+
     forward!(pub fn note(&mut self, msg: &str) -> &mut Self);
     forward!(pub fn span_note<S: Into<MultiSpan>>(&mut self,
                                                   sp: S,
diff --git a/src/librustc_typeck/check/cast.rs b/src/librustc_typeck/check/cast.rs
@@ -425,22 +425,39 @@ impl<'a, 'gcx, 'tcx> CastCheck<'tcx> {
             self.report_cast_to_unsized_type(fcx);
         } else if self.expr_ty.references_error() || self.cast_ty.references_error() {
             // No sense in giving duplicate error messages
-        } else if self.try_coercion_cast(fcx) {
-            self.trivial_cast_lint(fcx);
-            debug!(" -> CoercionCast");
-            fcx.tables.borrow_mut().cast_kinds_mut().insert(self.expr.hir_id,
-                                                            CastKind::CoercionCast);
         } else {
-            match self.do_check(fcx) {
-                Ok(k) => {
-                    debug!(" -> {:?}", k);
-                    fcx.tables.borrow_mut().cast_kinds_mut().insert(self.expr.hir_id, k);
+            match self.try_coercion_cast(fcx) {
+                Ok(()) => {
+                    self.trivial_cast_lint(fcx);
+                    debug!(" -> CoercionCast");
+                    fcx.tables.borrow_mut().cast_kinds_mut()
+                        .insert(self.expr.hir_id, CastKind::CoercionCast);
+                }
+                Err(ty::error::TypeError::ObjectUnsafeCoercion(did)) => {
+                    self.report_object_unsafe_cast(&fcx, did);
+                }
+                Err(_) => {
+                    match self.do_check(fcx) {
+                        Ok(k) => {
+                            debug!(" -> {:?}", k);
+                            fcx.tables.borrow_mut().cast_kinds_mut()
+                                .insert(self.expr.hir_id, k);
+                        }
+                        Err(e) => self.report_cast_error(fcx, e),
+                    };
                 }
-                Err(e) => self.report_cast_error(fcx, e),
             };
         }
     }
 
+    fn report_object_unsafe_cast(&self, fcx: &FnCtxt<'a, 'gcx, 'tcx>, did: DefId) {
+        let violations = fcx.tcx.global_tcx().object_safety_violations(did);
+        let mut err = fcx.tcx.report_object_safety_error(self.cast_span, did, violations);
+        err.note(&format!("required by cast to type '{}'",
+                          fcx.ty_to_string(self.cast_ty)));
+        err.emit();
+    }
+
     /// Check a cast, and report an error if one exists. In some cases, this
     /// can return Ok and create type errors in the fcx rather than returning
     /// directly. coercion-cast is handled in check instead of here.
@@ -636,8 +653,12 @@ impl<'a, 'gcx, 'tcx> CastCheck<'tcx> {
         }
     }
 
-    fn try_coercion_cast(&self, fcx: &FnCtxt<'a, 'gcx, 'tcx>) -> bool {
-        fcx.try_coerce(self.expr, self.expr_ty, self.cast_ty, AllowTwoPhase::No).is_ok()
+    fn try_coercion_cast(&self, fcx: &FnCtxt<'a, 'gcx, 'tcx>)
+                         -> Result<(), ty::error::TypeError> {
+        match fcx.try_coerce(self.expr, self.expr_ty, self.cast_ty, AllowTwoPhase::No) {
+            Ok(_) => Ok(()),
+            Err(err) => Err(err),
+        }
     }
 }
 
diff --git a/src/librustc_typeck/check/coercion.rs b/src/librustc_typeck/check/coercion.rs
@@ -186,9 +186,16 @@ impl<'f, 'gcx, 'tcx> Coerce<'f, 'gcx, 'tcx> {
         // a "spurious" type variable, and we don't want to have that
         // type variable in memory if the coercion fails.
         let unsize = self.commit_if_ok(|_| self.coerce_unsized(a, b));
-        if unsize.is_ok() {
-            debug!("coerce: unsize successful");
-            return unsize;
+        match unsize {
+            Ok(_) => {
+                debug!("coerce: unsize successful");
+                return unsize;
+            }
+            Err(TypeError::ObjectUnsafeCoercion(did)) => {
+                debug!("coerce: unsize not object safe");
+                return Err(TypeError::ObjectUnsafeCoercion(did));
+            }
+            Err(_) => {}
         }
         debug!("coerce: unsize failed");
 
@@ -657,9 +664,8 @@ impl<'f, 'gcx, 'tcx> Coerce<'f, 'gcx, 'tcx> {
                     );
                     match selcx.select(&obj_safe.with(pred.clone())) {
                         Ok(_) => (),
-                        Err(err) => {
-                            self.report_selection_error(&obj_safe, &err, false);
-                            return Err(TypeError::Mismatch)
+                        Err(_) => {
+                            return Err(TypeError::ObjectUnsafeCoercion(target_def_id))
                         }
                     }
                 }

Original file line number	Diff line number	Diff line change
`@@ -42,6 +42,7 @@ pub enum TypeError<'tcx> {`
`42`	`42`	`ProjectionMismatched(ExpectedFound<DefId>),`
`43`	`43`	`ProjectionBoundsLength(ExpectedFound<usize>),`
`44`	`44`	`ExistentialMismatch(ExpectedFound<&'tcx ty::List<ty::ExistentialPredicate<'tcx>>>),`
	`45`	`+ ObjectUnsafeCoercion(DefId),`
`45`	`46`	`}`
`46`	`47`
`47`	`48`	`#[derive(Clone, RustcEncodable, RustcDecodable, PartialEq, Eq, Hash, Debug, Copy)]`
`@@ -144,6 +145,7 @@ impl<'tcx> fmt::Display for TypeError<'tcx> {`
`144`	`145`	report_maybe_different(f, &format!("trait `{}`", values.expected),
`145`	`146`	&format!("trait `{}`", values.found))
`146`	`147`	`}`
	`148`	`+ ObjectUnsafeCoercion(_) => write!(f, "coercion to object-unsafe trait object"),`
`147`	`149`	`}`
`148`	`150`	`}`
`149`	`151`	`}`
Original file line number	Diff line number	Diff line change
`@@ -443,7 +443,8 @@ impl<'a, 'tcx> Lift<'tcx> for ty::error::TypeError<'a> {`
`443`	`443`	`ProjectionMismatched(x) => ProjectionMismatched(x),`
`444`	`444`	`ProjectionBoundsLength(x) => ProjectionBoundsLength(x),`
`445`	`445`	`Sorts(ref x) => return tcx.lift(x).map(Sorts),`
`446`		`- ExistentialMismatch(ref x) => return tcx.lift(x).map(ExistentialMismatch)`
	`446`	`+ ExistentialMismatch(ref x) => return tcx.lift(x).map(ExistentialMismatch),`
	`447`	`+ ObjectUnsafeCoercion(defid) => ObjectUnsafeCoercion(defid),`
`447`	`448`	`})`
`448`	`449`	`}`
`449`	`450`	`}`
`@@ -1031,6 +1032,7 @@ EnumTypeFoldableImpl! {`
`1031`	`1032`	`(ty::error::TypeError::ProjectionBoundsLength)(x),`
`1032`	`1033`	`(ty::error::TypeError::Sorts)(x),`
`1033`	`1034`	`(ty::error::TypeError::ExistentialMismatch)(x),`
	`1035`	`+ (ty::error::TypeError::ObjectUnsafeCoercion)(x),`
`1034`	`1036`	`}`
`1035`	`1037`	`}`
`1036`	`1038`