-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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.
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
Tracking issue for future-incompatibility lint where_clauses_object_safety
#51443
Comments
To clarify: we are specifically interested in getting feedback on places where this lint causes you issues, since that will help us to judge how well more precise fixes would work. |
This caused a future-compat warning in the upcoming #![feature(arbitrary_self_types, pin)]
use std::marker::Unpin;
use std::mem::PinMut;
trait Future {
fn poll(self: PinMut<Self>);
}
trait FutureExt: Future {
fn poll_unpin(&mut self) where Self: Unpin {
PinMut::new(self).poll()
}
} The error:
It seems like it should be possible to permit calls to |
Thanks @cramertj -- that falls pretty squarely into the rules we had in mind! Let's discuss the details of those rules in #50781 I guess, I want to try and "re-sync" (but I guess I'd rather keep this thread for people to report issues). But just to summarize, the rule we had in mind is roughly this:
That applies to your example, I believe, since This is a kind of generalization of the rule around |
For anyone reading this, I got the warning/error on a trait that was like this:
In my case I was able to make the problem go away by requiring the thing implementing Config to also implement serde::Serialize:
|
This broke bindgen:
This is on this bindgen commit fwiw: rust-lang/rust-bindgen@29dad40 |
To fix errors spawned since rust-lang/rust#51443.
I have an extension trait that was never supposed to be made into an object which triggers this. It only exists to add a few methods. I like this as a lint but the Edit: In fact, the lint is a false positive. The trait isn't dyn capable already because it contains generic functions. |
I bumped into this upcasting boxed trait objects with marker traits. There may be a way to do this safely that doesn't fall foul of this lint? use std::any::Any;
trait Trait: Any {
fn into_any(self: Box<Self>) -> Box<Any>;
fn into_any_send(self: Box<Self>) -> Box<Any+Send> where Self: Send;
fn into_any_sync(self: Box<Self>) -> Box<Any+Sync> where Self: Sync;
fn into_any_send_sync(self: Box<Self>) -> Box<Any+Send+Sync> where Self: Send+Sync;
}
impl<T> Trait for T where T: Any {
fn into_any(self: Box<Self>) -> Box<Any> {
self
}
fn into_any_send(self: Box<Self>) -> Box<Any+Send> where Self: Send {
self
}
fn into_any_sync(self: Box<Self>) -> Box<Any+Sync> where Self: Sync {
self
}
fn into_any_send_sync(self: Box<Self>) -> Box<Any+Send+Sync> where Self: Send+Sync {
self
}
}
fn main() {
let a: usize = 123;
let b: Box<Trait+Send+Sync> = Box::new(a);
let c: Box<Any+Send+Sync> = b.into_any_send_sync();
let _d: usize = *Box::<Any>::downcast(c).unwrap();
} |
This warns on
|
I have an extension trait that i want to be usable with ?Sized types (including str) where this is causing a problem. This is the reduced version of the trait:
With the previous 2 methods I get this somewhat confusing pair of warning and error :
It seems to me that if it knows that it can't be turned into a trait object because it has a generic method, |
On the AnyMap case: it’s substantially the same as what @alecmocatta is reporting. This lint causes trouble with auto traits.
AnyMap has an extension of that, a cloneable Any. That part is fine, but when you get to additional trait bounds, you need a function that will be in the vtable; and that’s where we run into trouble under this new scheme. Here’s a simplified version of some of the code (covering only the #[doc(hidden)]
pub trait CloneToAny {
/// Clone `self` into a new `Box<CloneAny>` object.
fn clone_to_any(&self) -> Box<CloneAny>;
/// Clone `self` into a new `Box<CloneAny + Send>` object.
fn clone_to_any_send(&self) -> Box<CloneAny + Send> where Self: Send;
}
impl<T: Any + Clone> CloneToAny for T {
fn clone_to_any(&self) -> Box<CloneAny> {
Box::new(self.clone())
}
fn clone_to_any_send(&self) -> Box<CloneAny + Send> where Self: Send {
Box::new(self.clone())
}
}
pub trait CloneAny: Any + CloneToAny { }
impl<T: Any + Clone> CloneAny for T { }
impl Clone for Box<CloneAny> {
fn clone(&self) -> Box<CloneAny> {
(**self).clone_to_any()
}
}
impl Clone for Box<CloneAny + Send> {
fn clone(&self) -> Box<CloneAny + Send> {
(**self).clone_to_any_send()
}
} Now as it stands, I know that if I have a I would like to retain the same interface: adding trait bounds is a known and sane way of handling these things—the way you’re supposed to handle it. But if necessary, I suppose the proliferation of traits can begin, with As it stands, I’m seeing a few options for me as a developer:
Expanding on the appeal: I don’t know, but my impression is that auto traits won’t trigger the memory unsafety problem. If so, is it reasonable to check whether the additional bounds on Self are all auto traits, and consider that acceptable code? And if tweaking the lint is not reasonable, is what I outline in the second item reasonable? Will Rust eat my laundry if I transmute (Quite incidentally: where I’m from “laundry” refers to the room where clothes washing is done, not to the pile of clothes, which is called the “washing”. The whole “may eat your laundry” thing was therefore much more amusing to contemplate than it would have been if it were mere clothes that were being devoured.) |
This is effectively what's done by the stdlib here: Lines 455 to 517 in f1aefb4
|
This is true, this lint can be tweaked to not fire if the bounds are all auto traits. |
Refering to Self in where clause became illegal because: > As was discovered in #50781 a combination of implementing a trait > directly for a dyn type and where clauses involving Self can punch a > hole in our dyn-capability rules. See this issue for details: rust-lang/rust#51443
How should I deal with that warning? Does it only trigger if I create a trait object or otherwise interact with |
I wanted to provide feedback that the warnings emitted by this in 1.57 (which are effectively unchanged from previous comments) when
As @itowlson pointed out in previous comments, this is an issue with how In other compiler diagnostics, when an issue involves a macro, the compiler can give me the hint that a macro is in play. This can often short-circuit a lot of debugging. Would it be possible for the compiler to detect when this issue is (possibly) influenced via a macro and provide such a hint? Furthermore, the existing warning says e.g.
However, my trait member is defined as |
I'm encoutering the same issue: https://github.com/lunar-mining/trait_ptr When I add Sync to the trait it compiles fine providing the trait methods pass This compiles without the warning:
This does not:
This is the full warning:
|
solved a bug which warned Session could not be made into an object. documented here: rust-lang/rust#51443 and here: https://stackoverflow.com/questions/72838225/rust-trait-warning-method-references-the-self-type-in-its-where-clause
solved a bug which warned Session could not be made into an object. documented here: rust-lang/rust#51443 and here: https://stackoverflow.com/questions/72838225/rust-trait-warning-method-references-the-self-type-in-its-where-clause
Did something happen intentionally with this recently? The following code compiles cleanly on nightly-2022-12-28, but triggers a warning with nightly-2022-12-29. Yet I don't see any recent PR linked to this tracking issue. pub trait Trait {
fn method(&self) where Self: Sync;
} warning: the trait `Trait` cannot be made into an object
--> src/main.rs:2:8
|
2 | fn method(&self) where Self: Sync;
| ^^^^^^
|
= warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
= note: for more information, see issue #51443 <https://github.com/rust-lang/rust/issues/51443>
note: for a trait to be "object safe" 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/main.rs:2:8
|
1 | pub trait Trait {
| ----- this trait cannot be made into an object...
2 | fn method(&self) where Self: Sync;
| ^^^^^^ ...because method `method` references the `Self` type in its `where` clause
= help: consider moving `method` to another trait
= note: `#[warn(where_clauses_object_safety)]` on by default |
This has also recently started triggering on |
Triggering on The triggering offending code: /// Encode the value part of a Tag-Length-Value encoded field, sans the [`Tag`]
/// and [`Length`].
pub trait EncodeValue {
/// Get the [`Header`] used to encode this value.
fn header(&self) -> Result<Header>
where
Self: Tagged,
{
Header::new(self.tag(), self.value_len()?)
}
/// Compute the length of this value (sans [`Tag`]+[`Length`] header) when
/// encoded as ASN.1 DER.
fn value_len(&self) -> Result<Length>;
/// Encode value (sans [`Tag`]+[`Length`] header) as ASN.1 DER using the
/// provided [`Writer`].
fn encode_value(&self, encoder: &mut dyn Writer) -> Result<()>;
} (Based on my reading of the object safety docs, the problem here is the |
Overall my impression is that this lint is too burdensome in the current form. Async_trait is a casualty if we keep this. For example: #[async_trait]
pub trait Trait {
fn synchronous(&self) {}
async fn asynchronous(&self) {}
} This needs to expand to something like: pub trait Trait {
// want to call this on non-Sync impls
fn synchronous(&self) {}
// as long as we're using Send futures (e.g. tokio) this only makes
// sense to call on Sync impls, hence the where-clause
fn asynchronous(&self) -> Pin<Box<dyn Future<Output = ()> + Send + '_>>
where
Self: Sync,
{
Box::pin(async move { ... })
}
} Using the following expansion instead is not a sufficient workaround because that prevents calling pub trait Trait: Sync {
fn synchronous(&self) {}
fn asynchronous(&self) -> Pin<Box<dyn Future<Output = ()> + Send + '_>> {
Box::pin(async move { ... })
}
} From studying #51443 (comment), it's not clear to me that the justification for this lint applies to the above trait in the first place. The motivation for the lint is that someone might write: unsafe impl Sync for dyn Trait + '_ {} and then improperly be able to call the
error[E0321]: cross-crate traits with a default impl, like `Sync`, can only be implemented for a struct/enum type, not `dyn Trait`
--> src/main.rs
|
| unsafe impl Sync for dyn Trait + '_ {}
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ can't implement cross-crate trait with a default impl for non-struct/enum type So I think this needs to be allowed. |
Well, we need to change something to fix #50781. Have there been other plausible proposals? |
Yes I tried to imply a counterproposal in the "2 reasons" in my comment.
|
Autotrait bounds on dyn-safe trait methods This PR is a successor to rust-lang#106604 implementing the approach encouraged by rust-lang#106604 (comment). **I propose making it legal to use autotraits as trait bounds on the `Self` type of trait methods in a trait object.** rust-lang#51443 (comment) justifies why this use case is particularly important in the context of the async-trait crate. ```rust #![feature(auto_traits)] #![deny(where_clauses_object_safety)] auto trait AutoTrait {} trait MyTrait { fn f(&self) where Self: AutoTrait; } fn main() { let _: &dyn MyTrait; } ``` Previously this would fail with: ```console error: the trait `MyTrait` cannot be made into an object --> src/main.rs:7:8 | 7 | fn f(&self) where Self: AutoTrait; | ^ | = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! = note: for more information, see issue rust-lang#51443 <rust-lang#51443> note: for a trait to be "object safe" 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/main.rs:7:8 | 6 | trait MyTrait { | ------- this trait cannot be made into an object... 7 | fn f(&self) where Self: AutoTrait; | ^ ...because method `f` references the `Self` type in its `where` clause = help: consider moving `f` to another trait ``` In order for this to be sound without hitting rust-lang#50781, **I further propose that we disallow handwritten autotrait impls that apply to trait objects.** Both of the following were previously allowed (_on nightly_) and no longer allowed in my proposal: ```rust auto trait AutoTrait {} trait MyTrait {} impl AutoTrait for dyn MyTrait {} // NOT ALLOWED impl<T: ?Sized> AutoTrait for T {} // NOT ALLOWED ``` (`impl<T> AutoTrait for T {}` remains allowed.) After this change, traits with a default impl are implemented for a trait object **if and only if** the autotrait is one of the trait object's trait bounds (or a supertrait of a bound). In other words `dyn Trait + AutoTrait` always implements AutoTrait while `dyn Trait` never implements AutoTrait. Fixes dtolnay/async-trait#228. r? `@lcnr`
Autotrait bounds on dyn-safe trait methods This PR is a successor to rust-lang#106604 implementing the approach encouraged by rust-lang#106604 (comment). **I propose making it legal to use autotraits as trait bounds on the `Self` type of trait methods in a trait object.** rust-lang#51443 (comment) justifies why this use case is particularly important in the context of the async-trait crate. ```rust #![feature(auto_traits)] #![deny(where_clauses_object_safety)] auto trait AutoTrait {} trait MyTrait { fn f(&self) where Self: AutoTrait; } fn main() { let _: &dyn MyTrait; } ``` Previously this would fail with: ```console error: the trait `MyTrait` cannot be made into an object --> src/main.rs:7:8 | 7 | fn f(&self) where Self: AutoTrait; | ^ | = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release! = note: for more information, see issue rust-lang#51443 <rust-lang#51443> note: for a trait to be "object safe" 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/main.rs:7:8 | 6 | trait MyTrait { | ------- this trait cannot be made into an object... 7 | fn f(&self) where Self: AutoTrait; | ^ ...because method `f` references the `Self` type in its `where` clause = help: consider moving `f` to another trait ``` In order for this to be sound without hitting rust-lang#50781, **I further propose that we disallow handwritten autotrait impls that apply to trait objects.** Both of the following were previously allowed (_on nightly_) and no longer allowed in my proposal: ```rust auto trait AutoTrait {} trait MyTrait {} impl AutoTrait for dyn MyTrait {} // NOT ALLOWED impl<T: ?Sized> AutoTrait for T {} // NOT ALLOWED ``` (`impl<T> AutoTrait for T {}` remains allowed.) After this change, traits with a default impl are implemented for a trait object **if and only if** the autotrait is one of the trait object's trait bounds (or a supertrait of a bound). In other words `dyn Trait + AutoTrait` always implements AutoTrait while `dyn Trait` never implements AutoTrait. Fixes dtolnay/async-trait#228. r? `@lcnr`
where_clauses_object_safety
future compatibility lintwhere_clauses_object_safety
This lint is no longer emitted by the compiler. |
This is the summary issue for the
WHERE_CLAUSES_OBJECT_SAFETY
future-compatibility warning and other related errors. The goal of this page is describe why this change was made and how you can fix code that is affected by it. It also provides a place to ask questions or register a complaint if you feel the change should not be made. For more information on the policy around future-compatibility warnings, see our [breaking change policy guidelines (https://github.com/rust-lang/rfcs/blob/master/text/1122-language-semver.md).What is the warning for?
As was discovered in #50781 a combination of implementing a trait directly for a
dyn
type and where clauses involvingSelf
can punch a hole in our dyn-capability rules rules. See the minimization:The fix applied in #50966 is to tighten the dyn-capability rules to make
X
not dyn-capable in this case, in general making any trait that containsSelf
in where clauses not dyn-capable, much like we disallowSelf
in arguments or return type. Few root regressions appeared in the crater run and those were fixable, though some crates being unmaintained complicates things.However that fix is heavy handed and disallows things we actually wish to support, still we went ahead with the warning as a stop gap while we look for a better, more targeted fix. The original issue contains some discussion of alternative fixes. With tight chalk integration, we could do some clever things.
Other alternatives discussed for the short-term:
impl Foo for dyn Bar + ..
whether that impl is causing trouble. This seems to be a dead end because it's hard to tell whichdyn
types are targeted by a blanket impl such asimpl<U: ?Sized + Bounds> Trait for U
.dyn Foo: Trait
then to castT
intodyn Foo
we require thatT: Trait
must also hold", probably for everyTrait
such thatSelf: Trait
appears in where clauses in the traits methods. This would alleviate the warning forSelf: Trait
where clauses. This may be practical to implement now, but seems subtle, we'd need to give it more thought.When will this warning become a hard error?
Hopefully we will develop a finer-grained rule and this warning will never be an error.
How to fix this?
Either drop the where clause or stop using
dyn
types with the affected trait. If this is not viable, that's probably ok, it's very unlikely that your code is actually unsound. But please do report your case here so take we may take it into consideration and see how to better support it!The text was updated successfully, but these errors were encountered: