Skip to content

Commit cd1f36b

Browse files
matthiaskrgrmatthiaskrgr
authored
Rollup merge of #133372 - cramertj:rework-dyn-suggestions, r=fmease
Refactor dyn-compatibility error and suggestions This CL makes a number of small changes to dyn compatibility errors: - "object safety" has been renamed to "dyn-compatibility" throughout - "Convert to enum" suggestions are no longer generated when there exists a type-generic impl of the trait or an impl for `dyn OtherTrait` - Several error messages are reorganized for user readability Additionally, the dyn compatibility error creation code has been split out into functions. cc #132713 cc #133267 r? `@compiler-errors`
2 parents dee7d0e + d00d4df commit cd1f36b

File tree

175 files changed

+1327
-1091
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

175 files changed

+1327
-1091
lines changed

‎compiler/rustc_error_codes/src/error_codes/E0038.md‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -264,15 +264,15 @@ trait Foo {
264264
### Trait contains associated constants
265265

266266
Just like static functions, associated constants aren't stored on the method
267-
table. If the trait or any subtrait contain an associated constant, they cannot
268-
be made into an object.
267+
table. If the trait or any subtrait contain an associated constant, they are not
268+
dyn compatible.
269269

270270
```compile_fail,E0038
271271
trait Foo {
272272
const X: i32;
273273
}
274274
275-
impl Foo {}
275+
impl dyn Foo {}
276276
```
277277

278278
A simple workaround is to use a helper method instead:

‎compiler/rustc_feature/src/removed.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ declare_features! (
163163
/// then removed. But there was no utility storing it separately, so now
164164
/// it's in this list.
165165
(removed, no_stack_check, "1.0.0", None, None),
166-
/// Allows making `dyn Trait` well-formed even if `Trait` is not dyn-compatible (object safe).
166+
/// Allows making `dyn Trait` well-formed even if `Trait` is not dyn compatible (object safe).
167167
/// Renamed to `dyn_compatible_for_dispatch`.
168168
(removed, object_safe_for_dispatch, "1.83.0", Some(43561),
169169
Some("renamed to `dyn_compatible_for_dispatch`")),

‎compiler/rustc_feature/src/unstable.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -272,7 +272,7 @@ declare_features! (
272272
(unstable, doc_notable_trait, "1.52.0", Some(45040)),
273273
/// Allows using the `may_dangle` attribute (RFC 1327).
274274
(unstable, dropck_eyepatch, "1.10.0", Some(34761)),
275-
/// Allows making `dyn Trait` well-formed even if `Trait` is not dyn-compatible[^1].
275+
/// Allows making `dyn Trait` well-formed even if `Trait` is not dyn compatible[^1].
276276
/// In that case, `dyn Trait: Trait` does not hold. Moreover, coercions and
277277
/// casts in safe Rust to `dyn Trait` for such a `Trait` is also forbidden.
278278
///

‎compiler/rustc_hir_analysis/src/coherence/mod.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,7 @@ fn check_object_overlap<'tcx>(
185185
// check for overlap with the automatic `impl Trait for dyn Trait`
186186
if let ty::Dynamic(data, ..) = trait_ref.self_ty().kind() {
187187
// This is something like `impl Trait1 for Trait2`. Illegal if
188-
// Trait1 is a supertrait of Trait2 or Trait2 is not dyn-compatible.
188+
// Trait1 is a supertrait of Trait2 or Trait2 is not dyn compatible.
189189

190190
let component_def_ids = data.iter().flat_map(|predicate| {
191191
match predicate.skip_binder() {

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

Lines changed: 108 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -433,34 +433,19 @@ pub fn report_dyn_incompatibility<'tcx>(
433433
hir::Node::Item(item) => Some(item.ident.span),
434434
_ => None,
435435
});
436+
436437
let mut err = struct_span_code_err!(
437438
tcx.dcx(),
438439
span,
439440
E0038,
440-
"the {} `{}` cannot be made into an object",
441+
"the {} `{}` is not dyn compatible",
441442
tcx.def_descr(trait_def_id),
442443
trait_str
443444
);
444-
err.span_label(span, format!("`{trait_str}` cannot be made into an object"));
445-
446-
if let Some(hir_id) = hir_id
447-
&& let hir::Node::Ty(ty) = tcx.hir_node(hir_id)
448-
&& let hir::TyKind::TraitObject([trait_ref, ..], ..) = ty.kind
449-
{
450-
let mut hir_id = hir_id;
451-
while let hir::Node::Ty(ty) = tcx.parent_hir_node(hir_id) {
452-
hir_id = ty.hir_id;
453-
}
454-
if tcx.parent_hir_node(hir_id).fn_sig().is_some() {
455-
// Do not suggest `impl Trait` when dealing with things like super-traits.
456-
err.span_suggestion_verbose(
457-
ty.span.until(trait_ref.span),
458-
"consider using an opaque type instead",
459-
"impl ",
460-
Applicability::MaybeIncorrect,
461-
);
462-
}
463-
}
445+
err.span_label(span, format!("`{trait_str}` is not dyn compatible"));
446+
447+
attempt_dyn_to_impl_suggestion(tcx, hir_id, &mut err);
448+
464449
let mut reported_violations = FxIndexSet::default();
465450
let mut multi_span = vec![];
466451
let mut messages = vec![];
@@ -475,7 +460,7 @@ pub fn report_dyn_incompatibility<'tcx>(
475460
if reported_violations.insert(violation.clone()) {
476461
let spans = violation.spans();
477462
let msg = if trait_span.is_none() || spans.is_empty() {
478-
format!("the trait cannot be made into an object because {}", violation.error_msg())
463+
format!("the trait is not dyn compatible because {}", violation.error_msg())
479464
} else {
480465
format!("...because {}", violation.error_msg())
481466
};
@@ -492,24 +477,20 @@ pub fn report_dyn_incompatibility<'tcx>(
492477
let has_multi_span = !multi_span.is_empty();
493478
let mut note_span = MultiSpan::from_spans(multi_span.clone());
494479
if let (Some(trait_span), true) = (trait_span, has_multi_span) {
495-
note_span.push_span_label(trait_span, "this trait cannot be made into an object...");
480+
note_span.push_span_label(trait_span, "this trait is not dyn compatible...");
496481
}
497482
for (span, msg) in iter::zip(multi_span, messages) {
498483
note_span.push_span_label(span, msg);
499484
}
500485
// FIXME(dyn_compat_renaming): Update the URL.
501486
err.span_note(
502487
note_span,
503-
"for a trait to be \"dyn-compatible\" it needs to allow building a vtable to allow the call \
504-
to be resolvable dynamically; for more information visit \
505-
<https://doc.rust-lang.org/reference/items/traits.html#object-safety>",
488+
"for a trait to be dyn compatible it needs to allow building a vtable\n\
489+
for more information, visit <https://doc.rust-lang.org/reference/items/traits.html#object-safety>",
506490
);
507491

508492
// Only provide the help if its a local trait, otherwise it's not actionable.
509493
if trait_span.is_some() {
510-
let mut reported_violations: Vec<_> = reported_violations.into_iter().collect();
511-
reported_violations.sort();
512-
513494
let mut potential_solutions: Vec<_> =
514495
reported_violations.into_iter().map(|violation| violation.solution()).collect();
515496
potential_solutions.sort();
@@ -520,68 +501,116 @@ pub fn report_dyn_incompatibility<'tcx>(
520501
}
521502
}
522503

504+
attempt_dyn_to_enum_suggestion(tcx, trait_def_id, &*trait_str, &mut err);
505+
506+
err
507+
}
508+
509+
/// Attempt to suggest converting the `dyn Trait` argument to an enumeration
510+
/// over the types that implement `Trait`.
511+
fn attempt_dyn_to_enum_suggestion(
512+
tcx: TyCtxt<'_>,
513+
trait_def_id: DefId,
514+
trait_str: &str,
515+
err: &mut Diag<'_>,
516+
) {
523517
let impls_of = tcx.trait_impls_of(trait_def_id);
524-
let impls = if impls_of.blanket_impls().is_empty() {
525-
impls_of
526-
.non_blanket_impls()
527-
.values()
528-
.flatten()
529-
.filter(|def_id| {
530-
!matches!(tcx.type_of(*def_id).instantiate_identity().kind(), ty::Dynamic(..))
531-
})
532-
.collect::<Vec<_>>()
533-
} else {
534-
vec![]
535-
};
536-
let externally_visible = if !impls.is_empty()
537-
&& let Some(def_id) = trait_def_id.as_local()
518+
519+
if !impls_of.blanket_impls().is_empty() {
520+
return;
521+
}
522+
523+
let concrete_impls: Option<Vec<Ty<'_>>> = impls_of
524+
.non_blanket_impls()
525+
.values()
526+
.flatten()
527+
.map(|impl_id| {
528+
// Don't suggest conversion to enum if the impl types have type parameters.
529+
// It's unlikely the user wants to define a generic enum.
530+
let Some(impl_type) = tcx.type_of(*impl_id).no_bound_vars() else { return None };
531+
532+
// Obviously unsized impl types won't be usable in an enum.
533+
// Note: this doesn't use `Ty::is_trivially_sized` because that function
534+
// defaults to assuming that things are *not* sized, whereas we want to
535+
// fall back to assuming that things may be sized.
536+
match impl_type.kind() {
537+
ty::Str | ty::Slice(_) | ty::Dynamic(_, _, ty::DynKind::Dyn) => {
538+
return None;
539+
}
540+
_ => {}
541+
}
542+
Some(impl_type)
543+
})
544+
.collect();
545+
let Some(concrete_impls) = concrete_impls else { return };
546+
547+
const MAX_IMPLS_TO_SUGGEST_CONVERTING_TO_ENUM: usize = 9;
548+
if concrete_impls.is_empty() || concrete_impls.len() > MAX_IMPLS_TO_SUGGEST_CONVERTING_TO_ENUM {
549+
return;
550+
}
551+
552+
let externally_visible = if let Some(def_id) = trait_def_id.as_local() {
538553
// We may be executing this during typeck, which would result in cycle
539554
// if we used effective_visibilities query, which looks into opaque types
540555
// (and therefore calls typeck).
541-
&& tcx.resolutions(()).effective_visibilities.is_exported(def_id)
542-
{
543-
true
556+
tcx.resolutions(()).effective_visibilities.is_exported(def_id)
544557
} else {
545558
false
546559
};
547-
match &impls[..] {
548-
[] => {}
549-
_ if impls.len() > 9 => {}
550-
[only] if externally_visible => {
551-
err.help(with_no_trimmed_paths!(format!(
552-
"only type `{}` is seen to implement the trait in this crate, consider using it \
553-
directly instead",
554-
tcx.type_of(*only).instantiate_identity(),
555-
)));
556-
}
557-
[only] => {
558-
err.help(with_no_trimmed_paths!(format!(
559-
"only type `{}` implements the trait, consider using it directly instead",
560-
tcx.type_of(*only).instantiate_identity(),
561-
)));
562-
}
563-
impls => {
564-
let types = impls
565-
.iter()
566-
.map(|t| {
567-
with_no_trimmed_paths!(format!(" {}", tcx.type_of(*t).instantiate_identity(),))
568-
})
569-
.collect::<Vec<_>>();
570-
err.help(format!(
571-
"the following types implement the trait, consider defining an enum where each \
572-
variant holds one of these types, implementing `{}` for this new enum and using \
573-
it instead:\n{}",
574-
trait_str,
575-
types.join("\n"),
576-
));
577-
}
560+
561+
if let [only_impl] = &concrete_impls[..] {
562+
let within = if externally_visible { " within this crate" } else { "" };
563+
err.help(with_no_trimmed_paths!(format!(
564+
"only type `{only_impl}` implements `{trait_str}`{within}; \
565+
consider using it directly instead."
566+
)));
567+
} else {
568+
let types = concrete_impls
569+
.iter()
570+
.map(|t| with_no_trimmed_paths!(format!(" {}", t)))
571+
.collect::<Vec<String>>()
572+
.join("\n");
573+
574+
err.help(format!(
575+
"the following types implement `{trait_str}`:\n\
576+
{types}\n\
577+
consider defining an enum where each variant holds one of these types,\n\
578+
implementing `{trait_str}` for this new enum and using it instead",
579+
));
578580
}
581+
579582
if externally_visible {
580583
err.note(format!(
581-
"`{trait_str}` can be implemented in other crates; if you want to support your users \
584+
"`{trait_str}` may be implemented in other crates; if you want to support your users \
582585
passing their own types here, you can't refer to a specific type",
583586
));
584587
}
588+
}
585589

586-
err
590+
/// Attempt to suggest that a `dyn Trait` argument or return type be converted
591+
/// to use `impl Trait`.
592+
fn attempt_dyn_to_impl_suggestion(tcx: TyCtxt<'_>, hir_id: Option<hir::HirId>, err: &mut Diag<'_>) {
593+
let Some(hir_id) = hir_id else { return };
594+
let hir::Node::Ty(ty) = tcx.hir_node(hir_id) else { return };
595+
let hir::TyKind::TraitObject([trait_ref, ..], ..) = ty.kind else { return };
596+
597+
// Only suggest converting `dyn` to `impl` if we're in a function signature.
598+
// This ensures that we don't suggest converting e.g.
599+
// `type Alias = Box<dyn DynIncompatibleTrait>;` to
600+
// `type Alias = Box<impl DynIncompatibleTrait>;`
601+
let Some((_id, first_non_type_parent_node)) =
602+
tcx.hir().parent_iter(hir_id).find(|(_id, node)| !matches!(node, hir::Node::Ty(_)))
603+
else {
604+
return;
605+
};
606+
if first_non_type_parent_node.fn_sig().is_none() {
607+
return;
608+
}
609+
610+
err.span_suggestion_verbose(
611+
ty.span.until(trait_ref.span),
612+
"consider using an opaque type instead",
613+
"impl ",
614+
Applicability::MaybeIncorrect,
615+
);
587616
}

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,6 @@ fn dyn_compatibility_violations(
5353
) -> &'_ [DynCompatibilityViolation] {
5454
debug_assert!(tcx.generics_of(trait_def_id).has_self);
5555
debug!("dyn_compatibility_violations: {:?}", trait_def_id);
56-
5756
tcx.arena.alloc_from_iter(
5857
elaborate::supertrait_def_ids(tcx, trait_def_id)
5958
.flat_map(|def_id| dyn_compatibility_violations_for_trait(tcx, def_id)),

‎tests/rustdoc-ui/invalid_const_in_lifetime_position.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,4 +8,4 @@ fn f<'a>(arg : Box<dyn X<Y<1> = &'a ()>>) {}
88
//~| ERROR associated type takes 0 generic arguments but 1 generic argument
99
//~| ERROR associated type takes 1 lifetime argument but 0 lifetime arguments
1010
//~| ERROR associated type takes 0 generic arguments but 1 generic argument
11-
//~| ERROR trait `X` cannot be made into an object
11+
//~| ERROR trait `X` is not dyn compatible

‎tests/rustdoc-ui/invalid_const_in_lifetime_position.stderr‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -92,17 +92,18 @@ LL | type Y<'a>;
9292
| ^
9393
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
9494

95-
error[E0038]: the trait `X` cannot be made into an object
95+
error[E0038]: the trait `X` is not dyn compatible
9696
--> $DIR/invalid_const_in_lifetime_position.rs:4:20
9797
|
9898
LL | fn f<'a>(arg : Box<dyn X<Y<1> = &'a ()>>) {}
99-
| ^^^^^^^^^^^^^^^^^^^^ `X` cannot be made into an object
99+
| ^^^^^^^^^^^^^^^^^^^^ `X` is not dyn compatible
100100
|
101-
note: for a trait to be "dyn-compatible" 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>
101+
note: for a trait to be dyn compatible it needs to allow building a vtable
102+
for more information, visit <https://doc.rust-lang.org/reference/items/traits.html#object-safety>
102103
--> $DIR/invalid_const_in_lifetime_position.rs:2:10
103104
|
104105
LL | trait X {
105-
| - this trait cannot be made into an object...
106+
| - this trait is not dyn compatible...
106107
LL | type Y<'a>;
107108
| ^ ...because it contains the generic associated type `Y`
108109
= help: consider moving `Y` to another trait

‎tests/rustdoc-ui/issues/ice-generic-type-alias-105742.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@ use std::ops::Index;
55
pub fn next<'a, T>(s: &'a mut dyn SVec<Item = T, Output = T>) {
66
//~^ expected 1 lifetime argument
77
//~| expected 1 generic argument
8-
//~| the trait `SVec` cannot be made into an object
9-
//~| `SVec` cannot be made into an object
8+
//~| the trait `SVec` is not dyn compatible
9+
//~| `SVec` is not dyn compatible
1010
//~| missing generics for associated type `SVec::Item`
1111
//~| missing generics for associated type `SVec::Item`
1212
let _ = s;

‎tests/rustdoc-ui/issues/ice-generic-type-alias-105742.stderr‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -294,19 +294,20 @@ help: add missing generic argument
294294
LL | Output = <Self as SVec>::Item> as SVec>::Item<T>,
295295
| +++
296296

297-
error[E0038]: the trait `SVec` cannot be made into an object
297+
error[E0038]: the trait `SVec` is not dyn compatible
298298
--> $DIR/ice-generic-type-alias-105742.rs:5:35
299299
|
300300
LL | pub fn next<'a, T>(s: &'a mut dyn SVec<Item = T, Output = T>) {
301-
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ `SVec` cannot be made into an object
301+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ `SVec` is not dyn compatible
302302
|
303-
note: for a trait to be "dyn-compatible" 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>
303+
note: for a trait to be dyn compatible it needs to allow building a vtable
304+
for more information, visit <https://doc.rust-lang.org/reference/items/traits.html#object-safety>
304305
--> $DIR/ice-generic-type-alias-105742.rs:15:17
305306
|
306307
LL | pub trait SVec: Index<
307308
| ____________----__^
308309
| | |
309-
| | this trait cannot be made into an object...
310+
| | this trait is not dyn compatible...
310311
LL | | <Self as SVec>::Item,
311312
... |
312313
LL | |/ Output = <Index<<Self as SVec>::Item,

0 commit comments

Comments
 (0)