-
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
Always encode RPITITs #109400
Always encode RPITITs #109400
Conversation
d948d77
to
288595b
Compare
I've also blessed a test that was a known bug and it's now working with the refactor. |
// [current] known-bug: #105197 | ||
// [current] failure-status:101 | ||
// [current] dont-check-compiler-stderr | ||
// [next] compile-flags: -Zlower-impl-trait-in-trait-to-assoc-ty |
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.
I am skeptical that this PR passes... I'll look into it though.
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.
Sorry, skeptical why this PR passes.
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.
Oh... this fails because we have no predicates_of
for RPITITs. That doesn't seem right 🤔
Let's leave this as not updated currently?
I don't think this needs to be on top of #109277 |
// [current] known-bug: #105197 | ||
// [current] failure-status:101 | ||
// [current] dont-check-compiler-stderr | ||
// [next] compile-flags: -Zlower-impl-trait-in-trait-to-assoc-ty |
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.
Sorry, skeptical why this PR passes.
@@ -1,3 +1,6 @@ | |||
// [next] compile-flags: -Zlower-impl-trait-in-trait-to-assoc-ty | |||
// revisions: current next |
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.
ok I looked into it -- we don't need revisions
here. We "inherit" the revisions from the parent. But it does inherit the revisions from the parent, so no extra file needed like in my other PR. Let me fix that.
// revisions: current next |
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.
we probably want the compile-flags
anyway, I'd say.
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.
In any case, feel free to close this PR and leave yours open that aggregates this stuff.
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.
Yes, the compile-flags is necessary to enable the -Z
flag when [next]
, but re-declaring "// revisions
" here does nothing, because the revisions is coming from the actual UI test (foreign.rs
)
The job Click to see the possible cause of the failure (guessed by this bot)
|
… r=spastorino RPITITs are `DefKind::Opaque` with new lowering strategy r? `@spastorino` Kinda cherry-picked rust-lang#109400
This fixes
tests/ui/impl-trait/in-trait/foreign.rs
because RPITIT'sdef_id
on the trait side were not being encoded in some cases.The only meaningful commit is d948d77
This is placed on top of #109277 but I'm not sure if that's needed, anyway #109277 is next in the queue.
r? @compiler-errors