Skip to content

Commit

Permalink
Auto merge of #117200 - rmehri01:repeated_help, r=WaffleLapkin
Browse files Browse the repository at this point in the history
Don't add redundant help for object safety violations

Fixes #117186

r? WaffleLapkin
  • Loading branch information
bors committed Nov 27, 2023
2 parents b4c4664 + ee96a7a commit 6eb9524
Show file tree
Hide file tree
Showing 4 changed files with 76 additions and 35 deletions.
13 changes: 10 additions & 3 deletions compiler/rustc_infer/src/traits/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,12 +101,19 @@ pub fn report_object_safety_error<'tcx>(
to be resolvable dynamically; for more information visit \
<https://doc.rust-lang.org/reference/items/traits.html#object-safety>",
);

// Only provide the help if its a local trait, otherwise it's not actionable.
if trait_span.is_some() {
let mut reported_violations: Vec<_> = reported_violations.into_iter().collect();
reported_violations.sort();
for violation in reported_violations {
// Only provide the help if its a local trait, otherwise it's not actionable.
violation.solution(&mut err);

let mut potential_solutions: Vec<_> =
reported_violations.into_iter().map(|violation| violation.solution()).collect();
potential_solutions.sort();
// Allows us to skip suggesting that the same item should be moved to another trait multiple times.
potential_solutions.dedup();
for solution in potential_solutions {
solution.add_to(&mut err);
}
}

Expand Down
95 changes: 65 additions & 30 deletions compiler/rustc_middle/src/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -832,50 +832,31 @@ impl ObjectSafetyViolation {
}
}

pub fn solution(&self, err: &mut Diagnostic) {
pub fn solution(&self) -> ObjectSafetyViolationSolution {
match self {
ObjectSafetyViolation::SizedSelf(_)
| ObjectSafetyViolation::SupertraitSelf(_)
| ObjectSafetyViolation::SupertraitNonLifetimeBinder(..) => {}
| ObjectSafetyViolation::SupertraitNonLifetimeBinder(..) => {
ObjectSafetyViolationSolution::None
}
ObjectSafetyViolation::Method(
name,
MethodViolationCode::StaticMethod(Some((add_self_sugg, make_sized_sugg))),
_,
) => {
err.span_suggestion(
add_self_sugg.1,
format!(
"consider turning `{name}` into a method by giving it a `&self` argument"
),
add_self_sugg.0.to_string(),
Applicability::MaybeIncorrect,
);
err.span_suggestion(
make_sized_sugg.1,
format!(
"alternatively, consider constraining `{name}` so it does not apply to \
trait objects"
),
make_sized_sugg.0.to_string(),
Applicability::MaybeIncorrect,
);
}
) => ObjectSafetyViolationSolution::AddSelfOrMakeSized {
name: *name,
add_self_sugg: add_self_sugg.clone(),
make_sized_sugg: make_sized_sugg.clone(),
},
ObjectSafetyViolation::Method(
name,
MethodViolationCode::UndispatchableReceiver(Some(span)),
_,
) => {
err.span_suggestion(
*span,
format!("consider changing method `{name}`'s `self` parameter to be `&self`"),
"&Self",
Applicability::MachineApplicable,
);
}
) => ObjectSafetyViolationSolution::ChangeToRefSelf(*name, *span),
ObjectSafetyViolation::AssocConst(name, _)
| ObjectSafetyViolation::GAT(name, _)
| ObjectSafetyViolation::Method(name, ..) => {
err.help(format!("consider moving `{name}` to another trait"));
ObjectSafetyViolationSolution::MoveToAnotherTrait(*name)
}
}
}
Expand All @@ -899,6 +880,60 @@ impl ObjectSafetyViolation {
}
}

#[derive(Clone, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)]
pub enum ObjectSafetyViolationSolution {
None,
AddSelfOrMakeSized {
name: Symbol,
add_self_sugg: (String, Span),
make_sized_sugg: (String, Span),
},
ChangeToRefSelf(Symbol, Span),
MoveToAnotherTrait(Symbol),
}

impl ObjectSafetyViolationSolution {
pub fn add_to(self, err: &mut Diagnostic) {
match self {
ObjectSafetyViolationSolution::None => {}
ObjectSafetyViolationSolution::AddSelfOrMakeSized {
name,
add_self_sugg,
make_sized_sugg,
} => {
err.span_suggestion(
add_self_sugg.1,
format!(
"consider turning `{name}` into a method by giving it a `&self` argument"
),
add_self_sugg.0,
Applicability::MaybeIncorrect,
);
err.span_suggestion(
make_sized_sugg.1,
format!(
"alternatively, consider constraining `{name}` so it does not apply to \
trait objects"
),
make_sized_sugg.0,
Applicability::MaybeIncorrect,
);
}
ObjectSafetyViolationSolution::ChangeToRefSelf(name, span) => {
err.span_suggestion(
span,
format!("consider changing method `{name}`'s `self` parameter to be `&self`"),
"&Self",
Applicability::MachineApplicable,
);
}
ObjectSafetyViolationSolution::MoveToAnotherTrait(name) => {
err.help(format!("consider moving `{name}` to another trait"));
}
}
}
}

/// Reasons a method might not be object-safe.
#[derive(Clone, Debug, PartialEq, Eq, Hash, HashStable, PartialOrd, Ord)]
pub enum MethodViolationCode {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_trait_selection/src/traits/object_safety.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ fn lint_object_unsafe_trait(
);
if node.is_some() {
// Only provide the help if its a local trait, otherwise it's not
violation.solution(err);
violation.solution().add_to(err);
}
err
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ LL | fn test(&self) -> [u8; bar::<Self>()];
| |
| ...because method `test` references the `Self` type in its `where` clause
= help: consider moving `test` to another trait
= help: consider moving `test` to another trait
= help: only type `()` implements the trait, consider using it directly instead

error: aborting due to 1 previous error
Expand Down

0 comments on commit 6eb9524

Please sign in to comment.