Skip to content

Commit

Permalink
Don't assume traits used as type are trait objs
Browse files Browse the repository at this point in the history
  • Loading branch information
@VulnBandit
VulnBandit committed Oct 5, 2024
1 parent f7c8928 commit 6a66761
Show file tree
Hide file tree
Showing 45 changed files with 378 additions and 630 deletions.
51 changes: 23 additions & 28 deletions compiler/rustc_hir_analysis/src/hir_ty_lowering/lint.rs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use rustc_ast::TraitObjectSyntax;
use rustc_errors::codes::*;
use rustc_errors::{Diag, EmissionGuarantee, StashKey};
use rustc_errors::{Diag, EmissionGuarantee, ErrorGuaranteed, StashKey, Suggestions};
use rustc_hir as hir;
use rustc_hir::def::{DefKind, Res};
use rustc_lint_defs::Applicability;
Expand All @@ -15,13 +15,16 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
///
/// *Bare* trait object types are ones that aren't preceded by the keyword `dyn`.
/// In edition 2021 and onward we emit a hard error for them.
pub(super) fn prohibit_or_lint_bare_trait_object_ty(&self, self_ty: &hir::Ty<'_>) {
pub(super) fn prohibit_or_lint_bare_trait_object_ty(
&self,
self_ty: &hir::Ty<'_>,
) -> Option<ErrorGuaranteed> {
let tcx = self.tcx();

let hir::TyKind::TraitObject([poly_trait_ref, ..], _, TraitObjectSyntax::None) =
self_ty.kind
else {
return;
return None;
};

let in_path = match tcx.parent_hir_node(self_ty.hir_id) {
Expand Down Expand Up @@ -70,8 +73,8 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
}

if self_ty.span.edition().at_least_rust_2021() {
let msg = "trait objects must include the `dyn` keyword";
let label = "add `dyn` keyword before this trait";
let msg = "expected a type, found a trait";
let label = "you can add the `dyn` keyword if you want a trait object";
let mut diag =
rustc_errors::struct_span_code_err!(self.dcx(), self_ty.span, E0782, "{}", msg);
if self_ty.span.can_be_used_for_suggestions()
Expand All @@ -83,7 +86,15 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
// Check if the impl trait that we are considering is an impl of a local trait.
self.maybe_suggest_blanket_trait_impl(self_ty, &mut diag);
self.maybe_suggest_assoc_ty_bound(self_ty, &mut diag);
diag.stash(self_ty.span, StashKey::TraitMissingMethod);
if let Some(mut sugg) =
tcx.dcx().steal_non_err(self_ty.span, StashKey::AssociatedTypeSuggestion)
&& let Suggestions::Enabled(ref mut s1) = diag.suggestions
&& let Suggestions::Enabled(ref mut s2) = sugg.suggestions
{
s1.append(s2);
sugg.cancel();
}
diag.stash(self_ty.span, StashKey::TraitMissingMethod)
} else {
tcx.node_span_lint(BARE_TRAIT_OBJECTS, self_ty.hir_id, self_ty.span, |lint| {
lint.primary_message("trait objects without an explicit `dyn` are deprecated");
Expand All @@ -96,6 +107,7 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
}
self.maybe_suggest_blanket_trait_impl(self_ty, lint);
});
None
}
}

Expand Down Expand Up @@ -171,41 +183,31 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
// 1. Independent functions
// 2. Functions inside trait blocks
// 3. Functions inside impl blocks
let (sig, generics, owner) = match tcx.hir_node_by_def_id(parent_id) {
let (sig, generics) = match tcx.hir_node_by_def_id(parent_id) {
hir::Node::Item(hir::Item { kind: hir::ItemKind::Fn(sig, generics, _), .. }) => {
(sig, generics, None)
(sig, generics)
}
hir::Node::TraitItem(hir::TraitItem {
kind: hir::TraitItemKind::Fn(sig, _),
generics,
owner_id,
..
}) => (sig, generics, Some(tcx.parent(owner_id.to_def_id()))),
}) => (sig, generics),
hir::Node::ImplItem(hir::ImplItem {
kind: hir::ImplItemKind::Fn(sig, _),
generics,
owner_id,
..
}) => (sig, generics, Some(tcx.parent(owner_id.to_def_id()))),
}) => (sig, generics),
_ => return false,
};
let Ok(trait_name) = tcx.sess.source_map().span_to_snippet(self_ty.span) else {
return false;
};
let impl_sugg = vec![(self_ty.span.shrink_to_lo(), "impl ".to_string())];
let mut is_downgradable = true;

// Check if trait object is safe for suggesting dynamic dispatch.
let is_dyn_compatible = match self_ty.kind {
hir::TyKind::TraitObject(objects, ..) => {
objects.iter().all(|(o, _)| match o.trait_ref.path.res {
Res::Def(DefKind::Trait, id) => {
if Some(id) == owner {
// For recursive traits, don't downgrade the error. (#119652)
is_downgradable = false;
}
tcx.is_dyn_compatible(id)
}
Res::Def(DefKind::Trait, id) => tcx.is_dyn_compatible(id),
_ => false,
})
}
Expand Down Expand Up @@ -252,9 +254,6 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
suggestion,
Applicability::MachineApplicable,
);
} else if is_downgradable {
// We'll emit the dyn-compatibility error already, with a structured suggestion.
diag.downgrade_to_delayed_bug();
}
return true;
}
Expand All @@ -278,10 +277,6 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
);
if !is_dyn_compatible {
diag.note(format!("`{trait_name}` it is dyn-incompatible, so it can't be `dyn`"));
if is_downgradable {
// We'll emit the dyn-compatibility error already, with a structured suggestion.
diag.downgrade_to_delayed_bug();
}
} else {
// No ampersand in suggestion if it's borrowed already
let (dyn_str, paren_dyn_str) =
Expand Down
19 changes: 12 additions & 7 deletions compiler/rustc_hir_analysis/src/hir_ty_lowering/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2063,13 +2063,18 @@ impl<'tcx> dyn HirTyLowerer<'tcx> + '_ {
)
}
hir::TyKind::TraitObject(bounds, lifetime, repr) => {
self.prohibit_or_lint_bare_trait_object_ty(hir_ty);

let repr = match repr {
TraitObjectSyntax::Dyn | TraitObjectSyntax::None => ty::Dyn,
TraitObjectSyntax::DynStar => ty::DynStar,
};
self.lower_trait_object_ty(hir_ty.span, hir_ty.hir_id, bounds, lifetime, repr)
if let Some(guar) = self.prohibit_or_lint_bare_trait_object_ty(hir_ty) {
// Don't continue with type analysis if the `dyn` keyword is missing
// It generates confusing errors, especially if the user meant to use another
// keyword like `impl`
Ty::new_error(tcx, guar)
} else {
let repr = match repr {
TraitObjectSyntax::Dyn | TraitObjectSyntax::None => ty::Dyn,
TraitObjectSyntax::DynStar => ty::DynStar,
};
self.lower_trait_object_ty(hir_ty.span, hir_ty.hir_id, bounds, lifetime, repr)
}
}
// If we encounter a fully qualified path with RTN generics, then it must have
// *not* gone through `lower_ty_maybe_return_type_notation`, and therefore
Expand Down
20 changes: 8 additions & 12 deletions tests/ui/associated-type-bounds/suggest-assoc-ty-bound-on-eq-bound.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,28 +2,24 @@
//@ edition: 2021

fn f(_: impl Trait<T = Copy>) {}
//~^ ERROR trait objects must include the `dyn` keyword
//~| HELP add `dyn` keyword before this trait
//~^ ERROR expected a type, found a trait
//~| HELP you can add the `dyn` keyword if you want a trait object
//~| HELP you might have meant to write a bound here
//~| ERROR the trait `Copy` cannot be made into an object

fn g(_: impl Trait<T = std::fmt::Debug + Eq>) {}
//~^ ERROR trait objects must include the `dyn` keyword
//~| HELP add `dyn` keyword before this trait
//~^ ERROR expected a type, found a trait
//~| HELP you can add the `dyn` keyword if you want a trait object
//~| HELP you might have meant to write a bound here
//~| ERROR only auto traits can be used as additional traits in a trait object
//~| HELP consider creating a new trait
//~| ERROR the trait `Eq` cannot be made into an object

fn h(_: impl Trait<T<> = 'static + for<'a> Fn(&'a ())>) {}
//~^ ERROR trait objects must include the `dyn` keyword
//~| HELP add `dyn` keyword before this trait
//~^ ERROR expected a type, found a trait
//~| HELP you can add the `dyn` keyword if you want a trait object
//~| HELP you might have meant to write a bound here

// Don't suggest assoc ty bound in trait object types, that's not valid:
type Obj = dyn Trait<T = Clone>;
//~^ ERROR trait objects must include the `dyn` keyword
//~| HELP add `dyn` keyword before this trait
//~^ ERROR expected a type, found a trait
//~| HELP you can add the `dyn` keyword if you want a trait object

trait Trait { type T; }

Expand Down
58 changes: 13 additions & 45 deletions tests/ui/associated-type-bounds/suggest-assoc-ty-bound-on-eq-bound.stderr
View file
Original file line number Diff line number Diff line change
@@ -1,41 +1,10 @@
error[E0038]: the trait `Copy` cannot be made into an object
--> $DIR/suggest-assoc-ty-bound-on-eq-bound.rs:4:20
|
LL | fn f(_: impl Trait<T = Copy>) {}
| ^^^^^^^^ `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 "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>

error[E0225]: only auto traits can be used as additional traits in a trait object
--> $DIR/suggest-assoc-ty-bound-on-eq-bound.rs:10:42
|
LL | fn g(_: impl Trait<T = std::fmt::Debug + Eq>) {}
| --------------- ^^ additional non-auto trait
| |
| first non-auto trait
|
= help: consider creating a new trait with all of these as supertraits and using that trait here instead: `trait NewTrait: Debug + Eq {}`
= note: auto-traits like `Send` and `Sync` are traits that have special properties; for more information on them, visit <https://doc.rust-lang.org/reference/special-types-and-traits.html#auto-traits>

error[E0038]: the trait `Eq` cannot be made into an object
--> $DIR/suggest-assoc-ty-bound-on-eq-bound.rs:10:24
|
LL | fn g(_: impl Trait<T = std::fmt::Debug + Eq>) {}
| ^^^^^^^^^^^^^^^^^^^^ `Eq` cannot be made into an object
|
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>
--> $SRC_DIR/core/src/cmp.rs:LL:COL
|
= note: the trait cannot be made into an object because it uses `Self` as a type parameter

error[E0782]: trait objects must include the `dyn` keyword
error[E0782]: expected a type, found a trait
--> $DIR/suggest-assoc-ty-bound-on-eq-bound.rs:4:24
|
LL | fn f(_: impl Trait<T = Copy>) {}
| ^^^^
|
help: add `dyn` keyword before this trait
help: you can add the `dyn` keyword if you want a trait object
|
LL | fn f(_: impl Trait<T = dyn Copy>) {}
| +++
Expand All @@ -44,13 +13,13 @@ help: you might have meant to write a bound here
LL | fn f(_: impl Trait<T: Copy>) {}
| ~

error[E0782]: trait objects must include the `dyn` keyword
--> $DIR/suggest-assoc-ty-bound-on-eq-bound.rs:10:24
error[E0782]: expected a type, found a trait
--> $DIR/suggest-assoc-ty-bound-on-eq-bound.rs:9:24
|
LL | fn g(_: impl Trait<T = std::fmt::Debug + Eq>) {}
| ^^^^^^^^^^^^^^^^^^^^
|
help: add `dyn` keyword before this trait
help: you can add the `dyn` keyword if you want a trait object
|
LL | fn g(_: impl Trait<T = dyn std::fmt::Debug + Eq>) {}
| +++
Expand All @@ -59,13 +28,13 @@ help: you might have meant to write a bound here
LL | fn g(_: impl Trait<T: std::fmt::Debug + Eq>) {}
| ~

error[E0782]: trait objects must include the `dyn` keyword
--> $DIR/suggest-assoc-ty-bound-on-eq-bound.rs:18:26
error[E0782]: expected a type, found a trait
--> $DIR/suggest-assoc-ty-bound-on-eq-bound.rs:14:26
|
LL | fn h(_: impl Trait<T<> = 'static + for<'a> Fn(&'a ())>) {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
help: add `dyn` keyword before this trait
help: you can add the `dyn` keyword if you want a trait object
|
LL | fn h(_: impl Trait<T<> = dyn 'static + for<'a> Fn(&'a ())>) {}
| +++
Expand All @@ -74,18 +43,17 @@ help: you might have meant to write a bound here
LL | fn h(_: impl Trait<T<>: 'static + for<'a> Fn(&'a ())>) {}
| ~

error[E0782]: trait objects must include the `dyn` keyword
--> $DIR/suggest-assoc-ty-bound-on-eq-bound.rs:24:26
error[E0782]: expected a type, found a trait
--> $DIR/suggest-assoc-ty-bound-on-eq-bound.rs:20:26
|
LL | type Obj = dyn Trait<T = Clone>;
| ^^^^^
|
help: add `dyn` keyword before this trait
help: you can add the `dyn` keyword if you want a trait object
|
LL | type Obj = dyn Trait<T = dyn Clone>;
| +++

error: aborting due to 7 previous errors
error: aborting due to 4 previous errors

Some errors have detailed explanations: E0038, E0225, E0782.
For more information about an error, try `rustc --explain E0038`.
For more information about this error, try `rustc --explain E0782`.
4 changes: 1 addition & 3 deletions ...eric_const_exprs/expected-type-of-closure-body-to-be-a-closure-or-coroutine-ice-113776.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,8 @@ fn f<T>(
1
}],
) -> impl Iterator<Item = SubAssign> {
//~^ ERROR the type parameter `Rhs` must be explicitly specified
//~^ ERROR expected a type, found a trait
//~| ERROR `()` is not an iterator
//~| ERROR trait objects must include the `dyn` keyword
//~| ERROR the type parameter `Rhs` must be explicitly specified [E0393]
}

pub fn main() {}
39 changes: 4 additions & 35 deletions ..._const_exprs/expected-type-of-closure-body-to-be-a-closure-or-coroutine-ice-113776.stderr
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,37 +16,6 @@ help: you might be missing a type parameter
LL | fn f<T, F>(
| +++

error[E0393]: the type parameter `Rhs` must be explicitly specified
--> $DIR/expected-type-of-closure-body-to-be-a-closure-or-coroutine-ice-113776.rs:16:27
|
LL | ) -> impl Iterator<Item = SubAssign> {
| ^^^^^^^^^
--> $SRC_DIR/core/src/ops/arith.rs:LL:COL
|
= note: type parameter `Rhs` must be specified for this
|
= note: because of the default `Self` reference, type parameters must be specified on object types
help: set the type parameter to the desired type
|
LL | ) -> impl Iterator<Item = SubAssign<Rhs>> {
| +++++

error[E0393]: the type parameter `Rhs` must be explicitly specified
--> $DIR/expected-type-of-closure-body-to-be-a-closure-or-coroutine-ice-113776.rs:16:27
|
LL | ) -> impl Iterator<Item = SubAssign> {
| ^^^^^^^^^
--> $SRC_DIR/core/src/ops/arith.rs:LL:COL
|
= note: type parameter `Rhs` must be specified for this
|
= note: because of the default `Self` reference, type parameters must be specified on object types
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`
help: set the type parameter to the desired type
|
LL | ) -> impl Iterator<Item = SubAssign<Rhs>> {
| +++++

error[E0277]: `()` is not an iterator
--> $DIR/expected-type-of-closure-body-to-be-a-closure-or-coroutine-ice-113776.rs:16:6
|
Expand All @@ -55,13 +24,13 @@ LL | ) -> impl Iterator<Item = SubAssign> {
|
= help: the trait `Iterator` is not implemented for `()`

error[E0782]: trait objects must include the `dyn` keyword
error[E0782]: expected a type, found a trait
--> $DIR/expected-type-of-closure-body-to-be-a-closure-or-coroutine-ice-113776.rs:16:27
|
LL | ) -> impl Iterator<Item = SubAssign> {
| ^^^^^^^^^
|
help: add `dyn` keyword before this trait
help: you can add the `dyn` keyword if you want a trait object
|
LL | ) -> impl Iterator<Item = dyn SubAssign> {
| +++
Expand All @@ -70,7 +39,7 @@ help: you might have meant to write a bound here
LL | ) -> impl Iterator<Item: SubAssign> {
| ~

error: aborting due to 5 previous errors
error: aborting due to 3 previous errors

Some errors have detailed explanations: E0277, E0393, E0412, E0782.
Some errors have detailed explanations: E0277, E0412, E0782.
For more information about an error, try `rustc --explain E0277`.
3 changes: 1 addition & 2 deletions tests/ui/const-generics/not_wf_param_in_rpitit.rs
View file
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
//@ edition:2021

trait Trait<const N: Trait = bar> {
trait Trait<const N: dyn Trait = bar> {
//~^ ERROR: cannot find value `bar` in this scope
//~| ERROR: cycle detected when computing type of `Trait::N`
//~| ERROR: the trait `Trait` cannot be made into an object
//~| ERROR: the trait `Trait` cannot be made into an object
//~| ERROR: the trait `Trait` cannot be made into an object
//~| ERROR: trait objects must include the `dyn` keyword
async fn a() {}
}

Expand Down
Loading

0 comments on commit 6a66761

Please sign in to comment.