[wip] degenerate object safety check for crater #66037
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
|
@bors try |
bors
added a commit
that referenced
this pull request
Nov 2, 2019
…3, r=<try> [wip] degenerate object safety check for crater The purpose of this PR is just to be able to do a crater run to assess the impact of the proposed fix for #57893. With this fix: * traits that are implemented for unsized types are not dyn safe * unless they are marked `#[rustc_dyn]` (this would eventually be `dyn trait`) r? @nikomatsakis -- this doesn't need review, won't land in this form
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
☀️ Try build successful - checks-azure |
|
@craterbot check |
|
👌 Experiment ℹ️ 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
|
|
More analysis shows that there are 20 root regressions (full details):
|
|
Looking more closely at the traitobject case reveals that it arises from |
|
(Note that "root: unknown causes - 298 (214 gh, 84 crates.io) detected crates which regressed due to this" may contain other roots -- that means that the tool wasn't able to find a line such as "Could not compile |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
@rustbot try |
|
@bors try |
bors
added a commit
that referenced
this pull request
Nov 12, 2019
…3, r=<try> [wip] degenerate object safety check for crater The purpose of this PR is just to be able to do a crater run to assess the impact of the proposed fix for #57893. With this fix: * traits that are implemented for unsized types are not dyn safe * unless they are marked `#[rustc_dyn]` (this would eventually be `dyn trait`) r? @nikomatsakis -- this doesn't need review, won't land in this form
|
☀️ Try build successful - checks-azure |
|
@craterbot check |
|
👌 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
|
@craterbot abort @lqd tells me that @ecstatic-morse added a nice feature to only re-run experiments on a subset of crates, I'll try that |
|
🗑️ Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
|
@lqd I don't know how to adjust the priority, but that makes sense to me |
|
I'll take care of it. Adjusting priority: this is going to be a very short run (1h30 or so) so let's move it earlier in the queue |
|
📝 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
|
|
trait StreamX {
type Item;
}
trait TryStreamX: StreamX {
type Ok;
type Error;
}
impl<S, T, E> TryStreamX for S
where S: ?Sized + StreamX<Item = Result<T, E>>
{
type Ok = T;
type Error = E;
}should reproduce the |
|
@lqd thanks! And I guess the problem is that people are using The concern, presumably, is that we might someday support |
|
I'm not sure how much people use (I imagine it would require some kind of reasoning that ensures that there is never another impl for |
|
People definitely use |
|
Yes, the This felt like it could be common, but somehow didn't turn up in all of the previous runs (maybe another full crater run when the PR isn't WIP anymore could be interesting, but it seems unlikely it'll find more of this particular use case). A quick search yields no (We need |
|
OK. Of course, as we well know, not all code is on crates.io. I have to think whether there is some rule that can accept this case. |
|
@rustbot modify labels to +S-waiting-on-review -S-waiting-on-author |
|
r? @ghost |
|
I've got to revive this effort sometime. I am not sure if it makes sense to keep this PR open in the meantime though... |
|
Going to close this for now |
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.
The purpose of this PR is just to be able to do a crater run to assess the impact of the proposed fix for #57893.
With this fix:
#[rustc_dyn](this would eventually bedyn trait)r? @nikomatsakis -- this doesn't need review, won't land in this form