Make AsMut unambiguous #52
Conversation
Remove AsMut<Option<CompatLevel>> from the public API to avoid ambiguous types
that can lead to:
error[E0283]: type annotations needed for `&mut T`
--> src/ruleset.rs:306:9
|
306 | let _ = ruleset.as_mut();
| ^ ------ type must be known at this point
|
note: multiple `impl`s satisfying `ruleset::Ruleset: AsMut<_>` found
Add tests to detect potential future change.
This is less elegant that the AsMut autoderivated implementations, but
it is safer.
Fixes landlock-lsm#48
Signed-off-by: Mickaël Salaün <mic@digikod.net>
l0kod
changed the title
| pub(crate) mod private { | ||
| use crate::CompatLevel; | ||
|
|
||
| pub trait OptionCompatLevelMut { | ||
| fn as_option_compat_level_mut(&mut self) -> &mut Option<CompatLevel>; | ||
| } | ||
| } |
There was a problem hiding this comment.
This feels like it will be a compiler error at some point in the future.
There was a problem hiding this comment.
This feels like it will be a compiler error at some point in the future.
What exactly and why?
There was a problem hiding this comment.
The OptionCompatLevelMut is part of the public API as a constraint on Compatible, right? But it's not actually made public.
There is a private type in public interface warning in rustc, but I suppose it doesn't apply to this (yet?).
It's possible this will never get changed since it might be considered a "feature" to prevent others from implementing a trait defined in your library, but it still seems not ideal to me.
There was a problem hiding this comment.
That may be a hack but there is no other way to make this work without exposing internal APIs. I did use this trick some years ago and it's still working the same. I don't think Rust will change that without an alternative.
There was a problem hiding this comment.
It's up to you, but if I had to choose between the two solutions I'd tell downstream to just deal with it rather than introducing a hacky workaround like this. Especially since you'll also have to do this for every new AsRef thing you want to add in the future.
There was a problem hiding this comment.
Yep, that's not optimal, but we'll see if we need to add back the AsMut API.
Remove AsMut<Option> from the public API to avoid ambiguous types that can lead to:
Add tests to detect potential future change.
This is less elegant that the AsMut autoderivated implementations, but it is safer.
Fixes #48
Cc @cd-work