-
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
Self
in where clauses may not be object safe
#50966
Self
in where clauses may not be object safe
#50966
Conversation
@bors try -- going to want a "check-only" crater run |
src/test/ui/issue-50781.stderr
Outdated
--> $DIR/issue-50781.rs:21:6 | ||
| | ||
LL | impl Trait for dyn X {} | ||
| ^^^^^ the trait `X` cannot be made into an object |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huh, the span here is a bit odd, no? I feel like it should underline the dyn X
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Presumably independent from this PR though)
…t-safe, r=<try> `Self` in where clauses may not be object safe Needs crater, virtually certain to cause regressions. In #50781 it was discovered that our object safety rules are not sound because we allow `Self` in where clauses without restrain. This PR is a direct fix to the rules so that we disallow methods with unsound where clauses. This currently uses hard error to measure impact, but we will want to downgrade it to a future compat error. Fixes #50781. r? @nikomatsakis
☀️ Test successful - status-travis |
bors likes it, so ping @pietroalbini for a crater run. Check-only should do it. |
Check-only crater run started. It should be done in ~3 days. |
Hi @leodasvacas (crater requester), @nikomatsakis (PR reviewer)! Crater results are at: http://cargobomb-reports.s3.amazonaws.com/pr-50966/index.html. 'Blacklisted' crates (spurious failures etc) can be found here. If you see any spurious failures not on the list, please make a PR against that file. (interested observers: Crater is a tool for testing the impact of changes on the crates.io ecosystem. You can find out more at the repo if you're curious) |
Well, with 216 regressions we surely can't land an hard error. Dependency tree for the 193 crates
|
From a superficial analysis, the root regressions seem to be: Since |
d32cb06
to
d5ecf8f
Compare
I made it a warn-by-default. The lint level (always crate level) and span (infringing method rather than use site) are not great, but it seems like too much work to make them better. Because in the case of Ready for another review by @nikomatsakis. |
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 |
d5ecf8f
to
36aa9d4
Compare
We may be able to get the author — @reem iirc? — to hand us over the keys so we can issue a patch release, presuming we have a fix in mind. I've not had a chance to look in more depth yet. |
OK so these are the failures from #[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;
/// Clone `self` into a new `Box<CloneAny + Sync>` object.
fn clone_to_any_sync(&self) -> Box<CloneAny + Sync> where Self: Sync;
/// Clone `self` into a new `Box<CloneAny + Send + Sync>` object.
fn clone_to_any_send_sync(&self) -> Box<CloneAny + Send + Sync> where Self: Send + Sync;
} |
(cross-posted) So I was discussing this problem with myself (I don't think @leodasvacas is around =) on Discord. You can search for this GUID to find it: 3b75dc47-42f1-44da-a39e-562f5875e932 However, the TL;DR is that we don't have to forbid all where clauses like So I think we should loosen the conditions in the PR, which should help with regressions. |
It was recently discovered in rust-lang/rust#50781 that `Self: Trait` bounds in trait methods are really not object-safe. This will be made into a warning by rust-lang/rust#50966, and vulkano will be affected by the warning . Thankfully the fix looks simple, by just moving `fn len` from `BufferAccess` to being directly in `TypedBufferAccess`.
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 |
☔ The latest upstream changes (presumably #51448) made this pull request unmergeable. Please resolve the merge conflicts. |
281ee1c
to
697a66c
Compare
☔ The latest upstream changes (presumably #51382) made this pull request unmergeable. Please resolve the merge conflicts. |
Ping from triage @leodasvacas! It's been a while since we heard from you, will you have time to work on this again? |
This is virtually certain to cause regressions, needs crater. In rust-lang#50781 it was discovered that our object safety rules are not sound because we allow `Self` in where clauses without restrain. This PR is a direct fix to the rules so that we disallow methods with unsound where clauses. This currently uses hard error to measure impact, but we will want to downgrade it to a future compat error. Fixes rust-lang#50781. r? @nikomatsakis
697a66c
to
cc60e01
Compare
I've rebased this. I think we still want to land this, and then try to follow up with something better. |
@bors r+ |
📌 Commit cc60e01 has been approved by |
@bors r- Actually, I want to check something. @leodasvacas, do you know precisely what set of conditions are needed to trigger the lint? Does it only trigger if you try to convert one of the affected traits into a trait object? |
@bors r+ Looks like yes. |
📌 Commit cc60e01 has been approved by |
…t-safe, r=nikomatsakis `Self` in where clauses may not be object safe Needs crater, virtually certain to cause regressions. In #50781 it was discovered that our object safety rules are not sound because we allow `Self` in where clauses without restrain. This PR is a direct fix to the rules so that we disallow methods with unsound where clauses. This currently uses hard error to measure impact, but we will want to downgrade it to a future compat error. Part of #50781. r? @nikomatsakis
☀️ Test successful - status-appveyor, status-travis |
Needs crater, virtually certain to cause regressions.
In #50781 it was discovered that our object safety rules are not sound because we allow
Self
in where clauses without restrain. This PR is a direct fix to the rules so that we disallow methods with unsound where clauses.This currently uses hard error to measure impact, but we will want to downgrade it to a future compat error.
Part of #50781.
r? @nikomatsakis