Let unsafe traits and autotraits be used as bound in dyn-safe trait #106604
Conversation
|
r? @cjgillot (rustbot has picked a reviewer for you, use r? to override) |
rustbot
added
S-waiting-on-review
|
I tend to agree with the rationale for unsafe traits. For safe traits from foreign crates, the error may be a little surprising to the user. As the point of the lint is to remove an unsoundness, this PR has soundness implications, so I'd rather be prudent. |
cjgillot
added
the
I-types-nominated
|
|
cc #50781 for bookkeeping. |
My opinion is the other way: I agree with this relaxation for auto traits, because they may be specified in dyn (e.g. |
compiler-errors
added
the
needs-fcp
|
(touched on this in t-types triage https://rust-lang.zulipchat.com/#narrow/stream/326132-t-types.2Fmeetings/topic/2023-01-18.20meeting/near/322084489) I strongly disagree with allowing for unsafe traits but not safe traits. Adding the additional safety requirement "Implementations of this trait are forbidden from applying to trait objects while not applying to all For auto traits you're right that it is safe as auto traits cannot be implemented for trait objects. As described in #50781 (comment) that's not quite right for local auto traits, but I am definitely open to strengthen this requirement also also forbid it for local auto traits. That means that trait objects only implement an auto trait I am open to only making |
lcnr
removed
the
I-types-nominated
dtolnay
added
S-waiting-on-author
|
Thank you, I agree with the feedback and the proposed approach in #106604 (comment). I have opened a new PR #107082 implementing that approach. |
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`
This PR implements the observation from #51443 (comment) that the
where_clauses_object_safetyfuture compatibility lint is unnecessarily restrictive in the case ofwhere Self: Syncbounds for 2 reasons:As an unsafe trait, writing
unsafe impl Sync for dyn Trait + '_ {}in order to makeTrait'swhere Self: Syncmethods callable ondyn Traitis something that the user would have brought upon themselves. That impl is wrong.As an autotrait, they can't even write such an impl in the first place.
Fixes dtolnay/async-trait#228.