[crater] make where_clauses_object_safety forbid
#124305
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
compiler-errors
changed the title
where_clauses_object_safety forbidwhere_clauses_object_safety forbid
rustbot
added
S-waiting-on-review
|
@bors try |
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Apr 23, 2024
[crater] make `where_clauses_object_safety` forbid cc rust-lang#50781 r? lcnr
This comment has been minimized.
This comment has been minimized.
|
☀️ Try build successful - checks-actions |
|
@craterbot check |
|
👌 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
craterbot
added
S-waiting-on-crater
|
🚧 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
|
🎉 Experiment
|
craterbot
added
S-waiting-on-review
compiler-errors
commented
May 20, 2024
| @@ -2129,10 +2129,10 @@ declare_lint! { | |||
| /// [issue #51443]: https://github.com/rust-lang/rust/issues/51443 | |||
| /// [future-incompatible]: ../index.md#future-incompatible-lints | |||
| pub WHERE_CLAUSES_OBJECT_SAFETY, | |||
| Warn, | |||
| Forbid, | |||
There was a problem hiding this comment.
Most crater regressions are due to deny(future_compat) having bad interactions w forbid
|
@bors try |
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
May 20, 2024
[crater] make `where_clauses_object_safety` forbid cc rust-lang#50781 r? lcnr
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
☀️ Try build successful - checks-actions |
|
🚨 Error: missing desired crates: {"async-trait-fn"} 🆘 If you have any trouble with Crater please ping |
|
👌 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
craterbot
removed
the
S-waiting-on-review
craterbot
added
the
S-waiting-on-crater
|
@craterbot edit p=1 |
|
🚨 Error: failed to parse the command 🆘 If you have any trouble with Crater please ping |
|
@craterbot p=1 |
|
📝 Configuration of the ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
|
🚧 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
|
🎉 Experiment
|
craterbot
added
S-waiting-on-review
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Jun 3, 2024
…li-obk Make `WHERE_CLAUSES_OBJECT_SAFETY` a regular object safety violation #### The issue In rust-lang#50781, we have known about unsound `where` clauses in function arguments: ```rust trait Impossible {} trait Foo { fn impossible(&self) where Self: Impossible; } impl Foo for &() { fn impossible(&self) where Self: Impossible, {} } // `where` clause satisfied for the object, meaning that the function now *looks* callable. impl Impossible for dyn Foo {} fn main() { let x: &dyn Foo = &&(); x.impossible(); } ``` ... which currently segfaults at runtime because we try to call a method in the vtable that doesn't exist. :( #### What did u change This PR removes the `WHERE_CLAUSES_OBJECT_SAFETY` lint and instead makes it a regular object safety violation. I choose to make this into a hard error immediately rather than a `deny` because of the time that has passed since this lint was authored, and the single (1) regression (see below). That means that it's OK to mention `where Self: Trait` where clauses in your trait, but making such a trait into a `dyn Trait` object will report an object safety violation just like `where Self: Sized`, etc. ```rust trait Impossible {} trait Foo { fn impossible(&self) where Self: Impossible; // <~ This definition is valid, just not object-safe. } impl Foo for &() { fn impossible(&self) where Self: Impossible, {} } fn main() { let x: &dyn Foo = &&(); // <~ THIS is where we emit an error. } ``` #### Regressions From a recent crater run, there's only one crate that relies on this behavior: rust-lang#124305 (comment). The crate looks unmaintained and there seems to be no dependents. #### Further We may later choose to relax this (e.g. when the where clause is implied by the supertraits of the trait or something), but this is not something I propose to do in this FCP. For example, given: ``` trait Tr { fn f(&self) where Self: Blanket; } impl<T: ?Sized> Blanket for T {} ``` Proving that some placeholder `S` implements `S: Blanket` would be sufficient to prove that the same (blanket) impl applies for both `Concerete: Blanket` and `dyn Trait: Blanket`. Repeating here that I don't think we need to implement this behavior right now. ---- r? lcnr
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Jun 3, 2024
…-obk Make `WHERE_CLAUSES_OBJECT_SAFETY` a regular object safety violation #### The issue In rust-lang#50781, we have known about unsound `where` clauses in function arguments: ```rust trait Impossible {} trait Foo { fn impossible(&self) where Self: Impossible; } impl Foo for &() { fn impossible(&self) where Self: Impossible, {} } // `where` clause satisfied for the object, meaning that the function now *looks* callable. impl Impossible for dyn Foo {} fn main() { let x: &dyn Foo = &&(); x.impossible(); } ``` ... which currently segfaults at runtime because we try to call a method in the vtable that doesn't exist. :( #### What did u change This PR removes the `WHERE_CLAUSES_OBJECT_SAFETY` lint and instead makes it a regular object safety violation. I choose to make this into a hard error immediately rather than a `deny` because of the time that has passed since this lint was authored, and the single (1) regression (see below). That means that it's OK to mention `where Self: Trait` where clauses in your trait, but making such a trait into a `dyn Trait` object will report an object safety violation just like `where Self: Sized`, etc. ```rust trait Impossible {} trait Foo { fn impossible(&self) where Self: Impossible; // <~ This definition is valid, just not object-safe. } impl Foo for &() { fn impossible(&self) where Self: Impossible, {} } fn main() { let x: &dyn Foo = &&(); // <~ THIS is where we emit an error. } ``` #### Regressions From a recent crater run, there's only one crate that relies on this behavior: rust-lang#124305 (comment). The crate looks unmaintained and there seems to be no dependents. #### Further We may later choose to relax this (e.g. when the where clause is implied by the supertraits of the trait or something), but this is not something I propose to do in this FCP. For example, given: ``` trait Tr { fn f(&self) where Self: Blanket; } impl<T: ?Sized> Blanket for T {} ``` Proving that some placeholder `S` implements `S: Blanket` would be sufficient to prove that the same (blanket) impl applies for both `Concerete: Blanket` and `dyn Trait: Blanket`. Repeating here that I don't think we need to implement this behavior right now. ---- r? lcnr
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Jun 4, 2024
…-obk Make `WHERE_CLAUSES_OBJECT_SAFETY` a regular object safety violation #### The issue In rust-lang#50781, we have known about unsound `where` clauses in function arguments: ```rust trait Impossible {} trait Foo { fn impossible(&self) where Self: Impossible; } impl Foo for &() { fn impossible(&self) where Self: Impossible, {} } // `where` clause satisfied for the object, meaning that the function now *looks* callable. impl Impossible for dyn Foo {} fn main() { let x: &dyn Foo = &&(); x.impossible(); } ``` ... which currently segfaults at runtime because we try to call a method in the vtable that doesn't exist. :( #### What did u change This PR removes the `WHERE_CLAUSES_OBJECT_SAFETY` lint and instead makes it a regular object safety violation. I choose to make this into a hard error immediately rather than a `deny` because of the time that has passed since this lint was authored, and the single (1) regression (see below). That means that it's OK to mention `where Self: Trait` where clauses in your trait, but making such a trait into a `dyn Trait` object will report an object safety violation just like `where Self: Sized`, etc. ```rust trait Impossible {} trait Foo { fn impossible(&self) where Self: Impossible; // <~ This definition is valid, just not object-safe. } impl Foo for &() { fn impossible(&self) where Self: Impossible, {} } fn main() { let x: &dyn Foo = &&(); // <~ THIS is where we emit an error. } ``` #### Regressions From a recent crater run, there's only one crate that relies on this behavior: rust-lang#124305 (comment). The crate looks unmaintained and there seems to be no dependents. #### Further We may later choose to relax this (e.g. when the where clause is implied by the supertraits of the trait or something), but this is not something I propose to do in this FCP. For example, given: ``` trait Tr { fn f(&self) where Self: Blanket; } impl<T: ?Sized> Blanket for T {} ``` Proving that some placeholder `S` implements `S: Blanket` would be sufficient to prove that the same (blanket) impl applies for both `Concerete: Blanket` and `dyn Trait: Blanket`. Repeating here that I don't think we need to implement this behavior right now. ---- r? lcnr
github-actions bot
pushed a commit
to rust-lang/miri
that referenced
this pull request
Jun 5, 2024
Make `WHERE_CLAUSES_OBJECT_SAFETY` a regular object safety violation
#### The issue
In #50781, we have known about unsound `where` clauses in function arguments:
```rust
trait Impossible {}
trait Foo {
fn impossible(&self)
where
Self: Impossible;
}
impl Foo for &() {
fn impossible(&self)
where
Self: Impossible,
{}
}
// `where` clause satisfied for the object, meaning that the function now *looks* callable.
impl Impossible for dyn Foo {}
fn main() {
let x: &dyn Foo = &&();
x.impossible();
}
```
... which currently segfaults at runtime because we try to call a method in the vtable that doesn't exist. :(
#### What did u change
This PR removes the `WHERE_CLAUSES_OBJECT_SAFETY` lint and instead makes it a regular object safety violation. I choose to make this into a hard error immediately rather than a `deny` because of the time that has passed since this lint was authored, and the single (1) regression (see below).
That means that it's OK to mention `where Self: Trait` where clauses in your trait, but making such a trait into a `dyn Trait` object will report an object safety violation just like `where Self: Sized`, etc.
```rust
trait Impossible {}
trait Foo {
fn impossible(&self)
where
Self: Impossible; // <~ This definition is valid, just not object-safe.
}
impl Foo for &() {
fn impossible(&self)
where
Self: Impossible,
{}
}
fn main() {
let x: &dyn Foo = &&(); // <~ THIS is where we emit an error.
}
```
#### Regressions
From a recent crater run, there's only one crate that relies on this behavior: rust-lang/rust#124305 (comment). The crate looks unmaintained and there seems to be no dependents.
#### Further
We may later choose to relax this (e.g. when the where clause is implied by the supertraits of the trait or something), but this is not something I propose to do in this FCP.
For example, given:
```
trait Tr {
fn f(&self) where Self: Blanket;
}
impl<T: ?Sized> Blanket for T {}
```
Proving that some placeholder `S` implements `S: Blanket` would be sufficient to prove that the same (blanket) impl applies for both `Concerete: Blanket` and `dyn Trait: Blanket`.
Repeating here that I don't think we need to implement this behavior right now.
----
r? lcnr
flip1995
pushed a commit
to flip1995/rust-clippy
that referenced
this pull request
Jun 28, 2024
Make `WHERE_CLAUSES_OBJECT_SAFETY` a regular object safety violation
#### The issue
In #50781, we have known about unsound `where` clauses in function arguments:
```rust
trait Impossible {}
trait Foo {
fn impossible(&self)
where
Self: Impossible;
}
impl Foo for &() {
fn impossible(&self)
where
Self: Impossible,
{}
}
// `where` clause satisfied for the object, meaning that the function now *looks* callable.
impl Impossible for dyn Foo {}
fn main() {
let x: &dyn Foo = &&();
x.impossible();
}
```
... which currently segfaults at runtime because we try to call a method in the vtable that doesn't exist. :(
#### What did u change
This PR removes the `WHERE_CLAUSES_OBJECT_SAFETY` lint and instead makes it a regular object safety violation. I choose to make this into a hard error immediately rather than a `deny` because of the time that has passed since this lint was authored, and the single (1) regression (see below).
That means that it's OK to mention `where Self: Trait` where clauses in your trait, but making such a trait into a `dyn Trait` object will report an object safety violation just like `where Self: Sized`, etc.
```rust
trait Impossible {}
trait Foo {
fn impossible(&self)
where
Self: Impossible; // <~ This definition is valid, just not object-safe.
}
impl Foo for &() {
fn impossible(&self)
where
Self: Impossible,
{}
}
fn main() {
let x: &dyn Foo = &&(); // <~ THIS is where we emit an error.
}
```
#### Regressions
From a recent crater run, there's only one crate that relies on this behavior: rust-lang/rust#124305 (comment). The crate looks unmaintained and there seems to be no dependents.
#### Further
We may later choose to relax this (e.g. when the where clause is implied by the supertraits of the trait or something), but this is not something I propose to do in this FCP.
For example, given:
```
trait Tr {
fn f(&self) where Self: Blanket;
}
impl<T: ?Sized> Blanket for T {}
```
Proving that some placeholder `S` implements `S: Blanket` would be sufficient to prove that the same (blanket) impl applies for both `Concerete: Blanket` and `dyn Trait: Blanket`.
Repeating here that I don't think we need to implement this behavior right now.
----
r? lcnr
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
cc #50781
r? lcnr