Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit af6b84a

Browse files
committedNov 26, 2023
don't add redundant help for object safety violations
1 parent 4fd68eb commit af6b84a

File tree

4 files changed

+75
-35
lines changed

4 files changed

+75
-35
lines changed
 

‎compiler/rustc_infer/src/traits/error_reporting/mod.rs

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -104,9 +104,15 @@ pub fn report_object_safety_error<'tcx>(
104104
if trait_span.is_some() {
105105
let mut reported_violations: Vec<_> = reported_violations.into_iter().collect();
106106
reported_violations.sort();
107-
for violation in reported_violations {
108-
// Only provide the help if its a local trait, otherwise it's not actionable.
109-
violation.solution(&mut err);
107+
108+
// Only provide the help if its a local trait, otherwise it's not actionable.
109+
let mut potential_solutions: Vec<_> =
110+
reported_violations.into_iter().map(|violation| violation.solution()).collect();
111+
potential_solutions.sort();
112+
// Allows us to skip suggesting that the same item should be moved to another trait multiple times.
113+
potential_solutions.dedup();
114+
for solution in potential_solutions {
115+
solution.add_to(&mut err);
110116
}
111117
}
112118

‎compiler/rustc_middle/src/traits/mod.rs

Lines changed: 65 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -843,50 +843,31 @@ impl ObjectSafetyViolation {
843843
}
844844
}
845845

846-
pub fn solution(&self, err: &mut Diagnostic) {
846+
pub fn solution(&self) -> ObjectSafetyViolationSolution {
847847
match self {
848848
ObjectSafetyViolation::SizedSelf(_)
849849
| ObjectSafetyViolation::SupertraitSelf(_)
850-
| ObjectSafetyViolation::SupertraitNonLifetimeBinder(..) => {}
850+
| ObjectSafetyViolation::SupertraitNonLifetimeBinder(..) => {
851+
ObjectSafetyViolationSolution::None
852+
}
851853
ObjectSafetyViolation::Method(
852854
name,
853855
MethodViolationCode::StaticMethod(Some((add_self_sugg, make_sized_sugg))),
854856
_,
855-
) => {
856-
err.span_suggestion(
857-
add_self_sugg.1,
858-
format!(
859-
"consider turning `{name}` into a method by giving it a `&self` argument"
860-
),
861-
add_self_sugg.0.to_string(),
862-
Applicability::MaybeIncorrect,
863-
);
864-
err.span_suggestion(
865-
make_sized_sugg.1,
866-
format!(
867-
"alternatively, consider constraining `{name}` so it does not apply to \
868-
trait objects"
869-
),
870-
make_sized_sugg.0.to_string(),
871-
Applicability::MaybeIncorrect,
872-
);
873-
}
857+
) => ObjectSafetyViolationSolution::AddSelfOrMakeSized {
858+
name: *name,
859+
add_self_sugg: add_self_sugg.clone(),
860+
make_sized_sugg: make_sized_sugg.clone(),
861+
},
874862
ObjectSafetyViolation::Method(
875863
name,
876864
MethodViolationCode::UndispatchableReceiver(Some(span)),
877865
_,
878-
) => {
879-
err.span_suggestion(
880-
*span,
881-
format!("consider changing method `{name}`'s `self` parameter to be `&self`"),
882-
"&Self",
883-
Applicability::MachineApplicable,
884-
);
885-
}
866+
) => ObjectSafetyViolationSolution::ChangeToRefSelf(*name, *span),
886867
ObjectSafetyViolation::AssocConst(name, _)
887868
| ObjectSafetyViolation::GAT(name, _)
888869
| ObjectSafetyViolation::Method(name, ..) => {
889-
err.help(format!("consider moving `{name}` to another trait"));
870+
ObjectSafetyViolationSolution::MoveToAnotherTrait(*name)
890871
}
891872
}
892873
}
@@ -910,6 +891,60 @@ impl ObjectSafetyViolation {
910891
}
911892
}
912893

894+
#[derive(Clone, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)]
895+
pub enum ObjectSafetyViolationSolution {
896+
None,
897+
AddSelfOrMakeSized {
898+
name: Symbol,
899+
add_self_sugg: (String, Span),
900+
make_sized_sugg: (String, Span),
901+
},
902+
ChangeToRefSelf(Symbol, Span),
903+
MoveToAnotherTrait(Symbol),
904+
}
905+
906+
impl ObjectSafetyViolationSolution {
907+
pub fn add_to(self, err: &mut Diagnostic) {
908+
match self {
909+
ObjectSafetyViolationSolution::None => {}
910+
ObjectSafetyViolationSolution::AddSelfOrMakeSized {
911+
name,
912+
add_self_sugg,
913+
make_sized_sugg,
914+
} => {
915+
err.span_suggestion(
916+
add_self_sugg.1,
917+
format!(
918+
"consider turning `{name}` into a method by giving it a `&self` argument"
919+
),
920+
add_self_sugg.0,
921+
Applicability::MaybeIncorrect,
922+
);
923+
err.span_suggestion(
924+
make_sized_sugg.1,
925+
format!(
926+
"alternatively, consider constraining `{name}` so it does not apply to \
927+
trait objects"
928+
),
929+
make_sized_sugg.0,
930+
Applicability::MaybeIncorrect,
931+
);
932+
}
933+
ObjectSafetyViolationSolution::ChangeToRefSelf(name, span) => {
934+
err.span_suggestion(
935+
span,
936+
format!("consider changing method `{name}`'s `self` parameter to be `&self`"),
937+
"&Self",
938+
Applicability::MachineApplicable,
939+
);
940+
}
941+
ObjectSafetyViolationSolution::MoveToAnotherTrait(name) => {
942+
err.help(format!("consider moving `{name}` to another trait"));
943+
}
944+
}
945+
}
946+
}
947+
913948
/// Reasons a method might not be object-safe.
914949
#[derive(Clone, Debug, PartialEq, Eq, Hash, HashStable, PartialOrd, Ord)]
915950
pub enum MethodViolationCode {

‎compiler/rustc_trait_selection/src/traits/object_safety.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,7 @@ fn lint_object_unsafe_trait(
192192
);
193193
if node.is_some() {
194194
// Only provide the help if its a local trait, otherwise it's not
195-
violation.solution(err);
195+
violation.solution().add_to(err);
196196
}
197197
err
198198
},

‎tests/ui/const-generics/generic_const_exprs/object-safety-err-ret.stderr

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ LL | fn test(&self) -> [u8; bar::<Self>()];
1414
| |
1515
| ...because method `test` references the `Self` type in its `where` clause
1616
= help: consider moving `test` to another trait
17-
= help: consider moving `test` to another trait
1817
= help: only type `()` implements the trait, consider using it directly instead
1918

2019
error: aborting due to previous error

0 commit comments

Comments
 (0)
Please sign in to comment.