Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Suggest an enum as a possible option when a trait object can’t be created due to object safety #80194

Closed
alilleybrinker opened this issue Dec 19, 2020 · 2 comments · Fixed by #117132
Closed

Suggest an enum as a possible option when a trait object can’t be created due to object safety #80194

alilleybrinker opened this issue Dec 19, 2020 · 2 comments · Fixed by #117132
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` A-trait-system Area: Trait system C-enhancement Category: An issue proposing an enhancement or a PR with one. D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. D-papercut Diagnostics: An error or lint that needs small tweaks. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@alilleybrinker
Copy link
Contributor

When you can't create a trait object due to a trait not being object safe, you have a few options:

  • if you control the trait, try to make it object safe (medium difficulty)
  • if you don't control the trait, try to get the upstream owner to make it object safe or find an alternative (hard)
  • if the set of types you're trying to trait-objectify is closed, use an enum (easy)

Unfortunately, the error message for a trait not being object safe doesn't point you in the direction of the enum option, and unless you're pretty familiar with Rust you may not consider it.

Adjusting the error message for failure to create a trait object due to object safety issues to mention an enum as a possibility could help people resolve these issues. I have no data, but in my experience the enum path is often workable because I'm working with a finite set of variants that I know in advance.
@alilleybrinker
Copy link
Contributor Author

For context, this was opened at the suggestion of @estebank on Twitter.

@camelid camelid added A-diagnostics Area: Messages for errors, warnings, and lints A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` A-trait-system Area: Trait system C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 20, 2020
@alilleybrinker
Copy link
Contributor Author

On further reflection, my list of options was incomplete, as exploiting type-erasure a la erased-serde is also a possibility, see the example here. This involves, rather than making the trait object safe, making an erased version of the trait which is object safe, and then implementing the non-object-safe version of that trait for trait objects of the erased trait.

So T isn't object safe, but ET is, and you can implement T for Box<dyn ET> or &dyn ET> or whatever other trait object of ET you want, and it resolves the safety issue. Fun trick!

@estebank estebank added D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. D-papercut Diagnostics: An error or lint that needs small tweaks. labels Jan 12, 2021
estebank added a commit to estebank/rust that referenced this issue Oct 24, 2023
@estebank
On object safety error, mention new enum as alternative
6c77456 
When we encounter a `dyn Trait` that isn't object safe, look for its
implementors. If there's one, mention using it directly If there are
less than 9, mention the possibility of creating a new enum and using
that instead.

Fix rust-lang#80194.
@estebank estebank mentioned this issue Oct 24, 2023
Merged
estebank added a commit to estebank/rust that referenced this issue Oct 27, 2023
@estebank
On object safety error, mention new enum as alternative
6bdf4c9 
When we encounter a `dyn Trait` that isn't object safe, look for its
implementors. If there's one, mention using it directly If there are
less than 9, mention the possibility of creating a new enum and using
that instead.

Fix rust-lang#80194.
bors added a commit to rust-lang-ci/rust that referenced this issue Oct 30, 2023
@bors
Auto merge of rust-lang#117132 - estebank:issue-80194, r=petrochenkov
7ebea0f 
On object safety error, mention new enum as alternative

When we encounter a `dyn Trait` that isn't object safe, look for its implementors. If there's one, mention using it directly If there are less than 9, mention the possibility of creating a new enum and using that instead.

Fix rust-lang#80194.
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Oct 30, 2023
@GuillaumeGomez
Rollup merge of rust-lang#117132 - estebank:issue-80194, r=petrochenkov
c7e6301 
On object safety error, mention new enum as alternative

When we encounter a `dyn Trait` that isn't object safe, look for its implementors. If there's one, mention using it directly If there are less than 9, mention the possibility of creating a new enum and using that instead.

Fix rust-lang#80194.
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this issue Oct 30, 2023
@GuillaumeGomez
Rollup merge of rust-lang#117132 - estebank:issue-80194, r=petrochenkov
95de91b 
On object safety error, mention new enum as alternative

When we encounter a `dyn Trait` that isn't object safe, look for its implementors. If there's one, mention using it directly If there are less than 9, mention the possibility of creating a new enum and using that instead.

Fix rust-lang#80194.
@bors bors closed this as completed in 8c04999 Oct 30, 2023
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Oct 30, 2023
@rust-timer
Unrolled build for rust-lang#117132
33ef793 
Rollup merge of rust-lang#117132 - estebank:issue-80194, r=petrochenkov

On object safety error, mention new enum as alternative

When we encounter a `dyn Trait` that isn't object safe, look for its implementors. If there's one, mention using it directly If there are less than 9, mention the possibility of creating a new enum and using that instead.

Fix rust-lang#80194.
@staffik staffik mentioned this issue May 21, 2024
Merged
github-merge-queue bot pushed a commit to near/nearcore that referenced this issue Jun 10, 2024
@staffik
refactor: Wrap validator_signer with MutableConfigValue (#11372)
b8f08d9 
Part of: #11264
This PR should be no-op:
- convert `Signer` and `ValidatorSigner` traits into an enum
- wrap `ValidatorSigner` with `MutableConfigValue`

`MutableConfigValue` requires to implement `PartialEq` and `Clone`
traits. These traits are not object safe, thus cannot be derived for
`ValidatorSigner` trait. We need to convert `ValidatorSigner` trait into
an enum, similarly `Signer` trait.
That's also the solution recommended by Rust:
rust-lang/rust#80194

Unfortunately, this change requires a change in enormous number of
places, because the existing code mostly used
`InMemory(Validator)Signer` directly instead of `dyn (Validator)Signer`.
To minimize changes, I added traits like
`From<InMemoryValidatorSigner> for ValidatorSigner` so that it suffices
to add `.into()` in most cases.
staffik added a commit to near/nearcore that referenced this issue Jun 25, 2024
@staffik
refactor: Wrap validator_signer with MutableConfigValue (#11372)
aace5dd 
Part of: #11264
This PR should be no-op:
- convert `Signer` and `ValidatorSigner` traits into an enum
- wrap `ValidatorSigner` with `MutableConfigValue`

`MutableConfigValue` requires to implement `PartialEq` and `Clone`
traits. These traits are not object safe, thus cannot be derived for
`ValidatorSigner` trait. We need to convert `ValidatorSigner` trait into
an enum, similarly `Signer` trait.
That's also the solution recommended by Rust:
rust-lang/rust#80194

Unfortunately, this change requires a change in enormous number of
places, because the existing code mostly used
`InMemory(Validator)Signer` directly instead of `dyn (Validator)Signer`.
To minimize changes, I added traits like
`From<InMemoryValidatorSigner> for ValidatorSigner` so that it suffices
to add `.into()` in most cases.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-suggestion-diagnostics Area: Suggestions generated by the compiler applied by `cargo fix` A-trait-system Area: Trait system C-enhancement Category: An issue proposing an enhancement or a PR with one. D-newcomer-roadblock Diagnostics: Confusing error or lint; hard to understand for new users. D-papercut Diagnostics: An error or lint that needs small tweaks. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

On object safety error, mention new enum as alternative estebank/rust
3 participants
@alilleybrinker @estebank @camelid