Hello! While writing part of the migration guide for Bevy 0.14, I stumbled across this conversation. Since Rust does not yet support object-safe traits that use -> impl Trait, or in the case async fn, we've written a few type-erased versions. (E.g. AssetReader vs. ErasedAssetReader)
While reviewing the pull request that implemented this, one of the contributors mentioned how it would be beneficial if we could add a hint directing people to use dyn ErasedTrait instead of dyn Trait, since the latter would fail to compile.
With the stabilization of the #[diagnostic] namespace, what if #[diagnostic::not_object_safe] was added in scenarios when a trait cannot be used with dyn? (Specifically when E0038 is raised.)
Example
#[diagnostic::not_object_safe(
message = "{Self} is not object safe.",
note = "consider using `ErasedAssetReader` instead",
)]
pub trait AssetReader {
// ...
}
pub trait ErasedAssetReader {
// ...
}
let asset_reader: impl AssetReader = todo!();
// This would fail to compile, displaying the nice diagnostics from above.
let erased_asset_reader: &dyn AssetReader = &asset_reader;
// This would succeed.
let erased_asset_reader: &dyn ErasedAssetReader = &asset_reader;
Naming
I'm not sure about the name not_object_safe. Please bikeshed ideas if you have them! Some of mine are:
not_object_safe
not_dyn
on_dyn (Unclear)
on_invalid_dyn
on_not_object_safe
on_object_unsafe ("unsafe" may be confusing)
Requirements
Since this solves a specific problem, there are a few things that could be done to prevent it from being used incorrectly:
Only allow annotating traits.
Raise a warning if the trait is object safe, since the diagnostics would never be shown.
What should happen when the attribute is used on a trait that actually is object-safe?
The attribute could become a bit of a foot gun if it was usable on object-safe traits, because it would muddy the signal that the attribute is supposed to send. So it seems desirable then to disallow that.
This would then allow the compiler to verify that ErasedAssetReader exists and to give you the right path to it (e.g. bevy::asset::ErasedAssetReader) given the use statements you have written so far.
Then, the message and note variant is for when there is no simple swap to make, pointing out alternative designs, while the object_safe_alternative variant expands to the message and note you've given (but with item paths etc correctly annotated).
You'd need to supply a path to it that's valid from the current file. Saying crate::asset::ErasedAssetLoader would also be fine; the compiler is responsible for resolving paths to traits, and then back to paths in the final message, telling the user the fully qualified path to use, as it does for (e.g.) a method whose trait is not currently in scope.
Notes, yes, but not messages. If you use this, the message is always "{Self} is not object safe.", and there's a note "consider using Thing instead", where Thing is the item specified by the diagnostic.
But you should be able to add extra notes if it's not obvious how to translate between the two options, to make it easier on users - for example, you might use one to tell people the relationship between bevy::asset::meta::Settings and bevy::asset::meta::AssetMetaDyn to make it easier on users converting a dyn AssetLoader into dyn ErasedAssetLoader.
It should be checked to see if it's object safe, and rejected if it's not - this is meant to point you at the right thing to resolve E0038, and it would be unhelpful at best if switching to the "right" thing resulted in a different E0038 instance.
The field is optional, so that you can use this for cases where you're documenting alternatives to object safety, instead of pointing you at an object-safe trait you can use. It can also be repeated, if there's more than one alternative that you could use with different trade-offs.
And yes, I do expect this to be a bit harder to implement - the advantage is that it stops you making mistakes, because the compiler (a) can semantically verify that the alternative you propose is object safe, and (b) can handle paths for you, so that you get the "right" import offered to users, not an internal-only path.
I like the idea; mainly for suggesting a distinct dedicated dyn-safe trait.
Speaking of which: the "object safe[ty]" terminology is kind of a legacy one from the beginning of Rust: Rust does not really have "objects", or if it did, users would not necessarily think of dyn Traits as such. Nowadays the preferred term is dyn-safety[1] (example). So a tiny bikeshedding nit w.r.t. the naming of the diagnostic tool would be to try and go in that direction
and even then, "safety" is a far-fetched term, from the point of view of users. I'd personally rather talk of compatibility, as in, being dyn-compatible, or just dyn-able. But official parlance does stick to safety, so let's do that as well. ↩︎
i'm not sure what makes you think that, they're called "trait objects" and they use vtables just like C++ objects.
i would much prefer using established terms than pretending we've invented something completely new. for example, rust claims to not have try/catch, but catch_unwind provides the same functionality.
soon rust will basically have full (multiple!) inheritance too, once we can downcast trait objects into their supertraits.
Using established terms for something which is kinda-sorta similar in Rust, but not really the same thing, is the source of a lot of confusion to people learning the language. Lifetimes, references, objects, downcasting, ...
If the Rust concepts were completely new, it might actually be less confusing than something close enough to throw people off. I wrote this section because of all the times I've seen people get confused by what dyn Trait is or means (typically due to an OO background when it comes to "object" confusion).
I prefer "dyn-safe" as there's an immediate connection to dyn Trait. If dyn Trait didn't use to be spelled just Trait, the terminology probably would have been different from the start. As it is, I already try to not lead with "trait object" on URLO; people generally recognize dyn Trait but haven't necessarily heard "trait object" yet.
(I do still use "trait objects" somewhat frequently because there's not always another suitable noun that comes to mind. And probably also out of inertia.)
Someone should tell rustdoc (now putting "this trait is not object safe" on every applicable trait page) and the reference, then.
The term possibly should be replaced, but it certainly has not been replaced. And inconsistent terminology is more confusing to newcomers than weird terminology.
i guess it's probably just confusing to high-level OO programmers then, since i've had a decent amount of experience with it, and i found it fairly clear.
