Skip to content

Commit

Permalink
Tweak "object unsafe" errors
Browse files Browse the repository at this point in the history 
Fix #77598.
  • Loading branch information
@estebank
estebank committed Oct 20, 2020
1 parent 9832374 commit ae0e3d0
Show file tree
Hide file tree
Showing 57 changed files with 699 additions and 421 deletions.
37 changes: 22 additions & 15 deletions compiler/rustc_infer/src/traits/error_reporting/mod.rs
View file
Expand Up @@ -2,12 +2,12 @@ use super::ObjectSafetyViolation;

use crate::infer::InferCtxt;
use rustc_data_structures::fx::FxHashSet;
use rustc_errors::{struct_span_err, Applicability, DiagnosticBuilder};
use rustc_errors::{struct_span_err, DiagnosticBuilder};
use rustc_hir as hir;
use rustc_hir::def_id::DefId;
use rustc_middle::ty::TyCtxt;
use rustc_span::symbol::Symbol;
use rustc_span::Span;
use rustc_span::{MultiSpan, Span};
use std::fmt;

impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
Expand Down Expand Up @@ -57,7 +57,8 @@ pub fn report_object_safety_error(
err.span_label(span, format!("the trait `{}` cannot be made into an object", trait_str));

let mut reported_violations = FxHashSet::default();
let mut had_span_label = false;
let mut multi_span = vec![];
let mut messages = vec![];
for violation in violations {
if let ObjectSafetyViolation::SizedSelf(sp) = &violation {
if !sp.is_empty() {
Expand All @@ -71,31 +72,37 @@ pub fn report_object_safety_error(
let msg = if trait_span.is_none() || spans.is_empty() {
format!("the trait cannot be made into an object because {}", violation.error_msg())
} else {
had_span_label = true;
format!("...because {}", violation.error_msg())
};
if spans.is_empty() {
err.note(&msg);
} else {
for span in spans {
err.span_label(span, &msg);
multi_span.push(span);
messages.push(msg.clone());
}
}
match (trait_span, violation.solution()) {
(Some(_), Some((note, None))) => {
err.help(&note);
}
(Some(_), Some((note, Some((sugg, span))))) => {
err.span_suggestion(span, &note, sugg, Applicability::MachineApplicable);
}
if trait_span.is_some() {
// Only provide the help if its a local trait, otherwise it's not actionable.
_ => {}
violation.solution(&mut err);
}
}
}
if let (Some(trait_span), true) = (trait_span, had_span_label) {
err.span_label(trait_span, "this trait cannot be made into an object...");
let has_multi_span = !multi_span.is_empty();
let mut note_span = MultiSpan::from_spans(multi_span.clone());
if let (Some(trait_span), true) = (trait_span, has_multi_span) {
note_span
.push_span_label(trait_span, "this trait cannot be made into an object...".to_string());
}
for (span, msg) in multi_span.into_iter().zip(messages.into_iter()) {
note_span.push_span_label(span, msg);
}
err.span_note(
note_span,
"for a trait to be \"object safe\" it needs to allow building a vtable to allow the call \
to be resolvable dynamically; for more information visit \
<https://doc.rust-lang.org/reference/items/traits.html#object-safety>",
);

if tcx.sess.trait_methods_not_found.borrow().contains(&span) {
// Avoid emitting error caused by non-existing method (#58734)
Expand Down
74 changes: 54 additions & 20 deletions compiler/rustc_middle/src/traits/mod.rs
View file
Expand Up @@ -13,6 +13,7 @@ use crate::mir::interpret::ErrorHandled;
use crate::ty::subst::SubstsRef;
use crate::ty::{self, AdtKind, Ty, TyCtxt};

use rustc_errors::{Applicability, DiagnosticBuilder};
use rustc_hir as hir;
use rustc_hir::def_id::DefId;
use rustc_span::symbol::Symbol;
Expand Down Expand Up @@ -652,7 +653,7 @@ impl ObjectSafetyViolation {
.into()
}
}
ObjectSafetyViolation::Method(name, MethodViolationCode::StaticMethod(_), _) => {
ObjectSafetyViolation::Method(name, MethodViolationCode::StaticMethod(_, _, _), _) => {
format!("associated function `{}` has no `self` parameter", name).into()
}
ObjectSafetyViolation::Method(
Expand Down Expand Up @@ -686,32 +687,65 @@ impl ObjectSafetyViolation {
}
}

pub fn solution(&self) -> Option<(String, Option<(String, Span)>)> {
Some(match *self {
ObjectSafetyViolation::SizedSelf(_) | ObjectSafetyViolation::SupertraitSelf(_) => {
return None;
pub fn solution(&self, err: &mut DiagnosticBuilder<'_>) {
match *self {
ObjectSafetyViolation::SizedSelf(_) | ObjectSafetyViolation::SupertraitSelf(_) => {}
ObjectSafetyViolation::Method(
name,
MethodViolationCode::StaticMethod(sugg, self_span, has_args),
_,
) => {
err.span_suggestion(
self_span,
&format!(
"consider turning `{}` into a method by giving it a `&self` argument",
name
),
format!("&self{}", if has_args { ", " } else { "" }),
Applicability::MaybeIncorrect,
);
match sugg {
Some((sugg, span)) => {
err.span_suggestion(
span,
&format!(
"alternatively, consider constraining `{}` so it does not apply to \
trait objects",
name
),
sugg.to_string(),
Applicability::MaybeIncorrect,
);
}
None => {
err.help(&format!(
"consider turning `{}` into a method by giving it a `&self` \
argument or constraining it so it does not apply to trait objects",
name
));
}
}
}
ObjectSafetyViolation::Method(name, MethodViolationCode::StaticMethod(sugg), _) => (
format!(
"consider turning `{}` into a method by giving it a `&self` argument or \
constraining it so it does not apply to trait objects",
name
),
sugg.map(|(sugg, sp)| (sugg.to_string(), sp)),
),
ObjectSafetyViolation::Method(
name,
MethodViolationCode::UndispatchableReceiver,
span,
) => (
format!("consider changing method `{}`'s `self` parameter to be `&self`", name),
Some(("&Self".to_string(), span)),
),
) => {
err.span_suggestion(
span,
&format!(
"consider changing method `{}`'s `self` parameter to be `&self`",
name
),
"&Self".to_string(),
Applicability::MachineApplicable,
);
}
ObjectSafetyViolation::AssocConst(name, _)
| ObjectSafetyViolation::Method(name, ..) => {
(format!("consider moving `{}` to another trait", name), None)
err.help(&format!("consider moving `{}` to another trait", name));
}
})
}
}

pub fn spans(&self) -> SmallVec<[Span; 1]> {
Expand All @@ -735,7 +769,7 @@ impl ObjectSafetyViolation {
#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash, HashStable)]
pub enum MethodViolationCode {
/// e.g., `fn foo()`
StaticMethod(Option<(&'static str, Span)>),
StaticMethod(Option<(&'static str, Span)>, Span, bool /* has args */),

/// e.g., `fn foo(&self, x: Self)`
ReferencesSelfInput(usize),
Expand Down
70 changes: 42 additions & 28 deletions compiler/rustc_trait_selection/src/traits/object_safety.rs
View file
Expand Up @@ -13,15 +13,15 @@ use super::elaborate_predicates;
use crate::infer::TyCtxtInferExt;
use crate::traits::query::evaluate_obligation::InferCtxtExt;
use crate::traits::{self, Obligation, ObligationCause};
use rustc_errors::{Applicability, FatalError};
use rustc_errors::FatalError;
use rustc_hir as hir;
use rustc_hir::def_id::DefId;
use rustc_middle::ty::subst::{GenericArg, InternalSubsts, Subst};
use rustc_middle::ty::{self, Ty, TyCtxt, TypeFoldable, TypeVisitor, WithConstness};
use rustc_middle::ty::{Predicate, ToPredicate};
use rustc_session::lint::builtin::WHERE_CLAUSES_OBJECT_SAFETY;
use rustc_span::symbol::Symbol;
use rustc_span::Span;
use rustc_span::{MultiSpan, Span};
use smallvec::SmallVec;

use std::array;
Expand Down Expand Up @@ -112,33 +112,35 @@ fn object_safety_violations_for_trait(
tcx.def_path_str(trait_def_id)
));
let node = tcx.hir().get_if_local(trait_def_id);
let msg = if let Some(hir::Node::Item(item)) = node {
err.span_label(
let mut spans = MultiSpan::from_span(*span);
if let Some(hir::Node::Item(item)) = node {
spans.push_span_label(
item.ident.span,
"this trait cannot be made into an object...",
"this trait cannot be made into an object...".into(),
);
spans.push_span_label(
*span,
format!("...because {}", violation.error_msg()),
);
format!("...because {}", violation.error_msg())
} else {
format!(
"the trait cannot be made into an object because {}",
violation.error_msg()
)
spans.push_span_label(
*span,
format!(
"the trait cannot be made into an object because {}",
violation.error_msg()
),
);
};
err.span_label(*span, &msg);
match (node, violation.solution()) {
(Some(_), Some((note, None))) => {
err.help(&note);
}
(Some(_), Some((note, Some((sugg, span))))) => {
err.span_suggestion(
span,
&note,
sugg,
Applicability::MachineApplicable,
);
}
// Only provide the help if its a local trait, otherwise it's not actionable.
_ => {}
err.span_note(
spans,
"for a trait to be \"object safe\" it needs to allow building a vtable \
to allow the call to be resolvable dynamically; for more information \
visit <https://doc.rust-lang.org/reference/items/traits.html\
#object-safety>",
);
if node.is_some() {
// Only provide the help if its a local trait, otherwise it's not
violation.solution(&mut err);
}
err.emit();
},
Expand Down Expand Up @@ -385,6 +387,8 @@ fn virtual_call_violation_for_method<'tcx>(
trait_def_id: DefId,
method: &ty::AssocItem,
) -> Option<MethodViolationCode> {
let sig = tcx.fn_sig(method.def_id);

// The method's first parameter must be named `self`
if !method.fn_has_self_parameter {
// We'll attempt to provide a structured suggestion for `Self: Sized`.
Expand All @@ -395,11 +399,21 @@ fn virtual_call_violation_for_method<'tcx>(
[.., pred] => (", Self: Sized", pred.span().shrink_to_hi()),
},
);
return Some(MethodViolationCode::StaticMethod(sugg));
// Get the span pointing at where the `self` receiver should be.
let sm = tcx.sess.source_map();
let self_span = method.ident.span.to(tcx
.hir()
.span_if_local(method.def_id)
.unwrap_or_else(|| sm.next_point(method.ident.span))
.shrink_to_hi());
let self_span = sm.span_through_char(self_span, '(').shrink_to_hi();
return Some(MethodViolationCode::StaticMethod(
sugg,
self_span,
!sig.inputs().skip_binder().is_empty(),
));
}

let sig = tcx.fn_sig(method.def_id);

for (i, input_ty) in sig.skip_binder().inputs()[1..].iter().enumerate() {
if contains_illegal_self_type_reference(tcx, trait_def_id, input_ty) {
return Some(MethodViolationCode::ReferencesSelfInput(i));
Expand Down
12 changes: 7 additions & 5 deletions src/test/ui/associated-consts/associated-const-in-trait.stderr
View file
@@ -1,15 +1,17 @@
error[E0038]: the trait `Trait` cannot be made into an object
--> $DIR/associated-const-in-trait.rs:9:6
|
LL | trait Trait {
| ----- this trait cannot be made into an object...
LL | const N: usize;
| - ...because it contains this associated `const`
...
LL | impl dyn Trait {
| ^^^^^^^^^ the trait `Trait` cannot be made into an object
|
= help: consider moving `N` to another trait
note: for a trait to be "object safe" it needs to allow building a vtable to allow the call to be resolvable dynamically; for more information visit <https://doc.rust-lang.org/reference/items/traits.html#object-safety>
--> $DIR/associated-const-in-trait.rs:6:11
|
LL | trait Trait {
| ----- this trait cannot be made into an object...
LL | const N: usize;
| ^ ...because it contains this associated `const`

error: aborting due to previous error

Expand Down
12 changes: 7 additions & 5 deletions src/test/ui/associated-item/issue-48027.stderr
View file
@@ -1,15 +1,17 @@
error[E0038]: the trait `Bar` cannot be made into an object
--> $DIR/issue-48027.rs:6:6
|
LL | trait Bar {
| --- this trait cannot be made into an object...
LL | const X: usize;
| - ...because it contains this associated `const`
...
LL | impl dyn Bar {}
| ^^^^^^^ the trait `Bar` cannot be made into an object
|
= help: consider moving `X` to another trait
note: for a trait to be "object safe" it needs to allow building a vtable to allow the call to be resolvable dynamically; for more information visit <https://doc.rust-lang.org/reference/items/traits.html#object-safety>
--> $DIR/issue-48027.rs:2:11
|
LL | trait Bar {
| --- this trait cannot be made into an object...
LL | const X: usize;
| ^ ...because it contains this associated `const`

error[E0283]: type annotations needed
--> $DIR/issue-48027.rs:3:32
Expand Down
11 changes: 7 additions & 4 deletions src/test/ui/coherence/coherence-impl-trait-for-trait-object-safe.stderr
View file
@@ -1,14 +1,17 @@
error[E0038]: the trait `NotObjectSafe` cannot be made into an object
--> $DIR/coherence-impl-trait-for-trait-object-safe.rs:7:24
|
LL | trait NotObjectSafe { fn eq(&self, other: Self); }
| ------------- ---- ...because method `eq` references the `Self` type in this parameter
| |
| this trait cannot be made into an object...
LL | impl NotObjectSafe for dyn NotObjectSafe { }
| ^^^^^^^^^^^^^^^^^ the trait `NotObjectSafe` cannot be made into an object
|
= help: consider moving `eq` to another trait
note: for a trait to be "object safe" it needs to allow building a vtable to allow the call to be resolvable dynamically; for more information visit <https://doc.rust-lang.org/reference/items/traits.html#object-safety>
--> $DIR/coherence-impl-trait-for-trait-object-safe.rs:6:43
|
LL | trait NotObjectSafe { fn eq(&self, other: Self); }
| ------------- ^^^^ ...because method `eq` references the `Self` type in this parameter
| |
| this trait cannot be made into an object...

error: aborting due to previous error

Expand Down
1 change: 1 addition & 0 deletions src/test/ui/did_you_mean/trait-object-reference-without-parens-suggestion.stderr
View file
Expand Up @@ -17,6 +17,7 @@ LL | let _: &Copy + 'static;
| ^^^^^ the trait `Copy` cannot be made into an object
|
= note: the trait cannot be made into an object because it requires `Self: Sized`
= note: for a trait to be "object safe" it needs to allow building a vtable to allow the call to be resolvable dynamically; for more information visit <https://doc.rust-lang.org/reference/items/traits.html#object-safety>

error: aborting due to 3 previous errors

Expand Down
16 changes: 11 additions & 5 deletions src/test/ui/error-codes/E0033-teach.stderr
View file
Expand Up @@ -7,15 +7,21 @@ LL | let trait_obj: &dyn SomeTrait = SomeTrait;
error[E0038]: the trait `SomeTrait` cannot be made into an object
--> $DIR/E0033-teach.rs:8:20
|
LL | let trait_obj: &dyn SomeTrait = SomeTrait;
| ^^^^^^^^^^^^^^ the trait `SomeTrait` cannot be made into an object
|
note: for a trait to be "object safe" it needs to allow building a vtable to allow the call to be resolvable dynamically; for more information visit <https://doc.rust-lang.org/reference/items/traits.html#object-safety>
--> $DIR/E0033-teach.rs:4:8
|
LL | trait SomeTrait {
| --------- this trait cannot be made into an object...
LL | fn foo();
| --- ...because associated function `foo` has no `self` parameter
...
LL | let trait_obj: &dyn SomeTrait = SomeTrait;
| ^^^^^^^^^^^^^^ the trait `SomeTrait` cannot be made into an object
| ^^^ ...because associated function `foo` has no `self` parameter
help: consider turning `foo` into a method by giving it a `&self` argument
|
help: consider turning `foo` into a method by giving it a `&self` argument or constraining it so it does not apply to trait objects
LL | fn foo(&self);
| ^^^^^
help: alternatively, consider constraining `foo` so it does not apply to trait objects
|
LL | fn foo() where Self: Sized;
| ^^^^^^^^^^^^^^^^^
Expand Down

0 comments on commit ae0e3d0

Please sign in to comment.