-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Mark DefineOpaqueTypes::Yes
use-sites uncovered by tests as FIXMEs
#117369
Conversation
duplicate of #117348 ? 😅 |
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.
Hm... I actually wonder if we may just want to track these usage sites as issues, rather than FIXMEs in the code.
Ideally, I would like to have interested individuals (or maybe just myself) study the way that the code is being used and create tests from that (or conclude that tests cannot be created and switch them to DefineOpaqueTypes::Yes
).
Also, each usage site alone is a bit too granular. Except for candidate assembly and selection in the new trait solver, we probably want to group (e.g.) all of the method probe usages together, and all of the coercion usages together, etc.
@@ -165,6 +165,7 @@ fn visit_implementation_of_dispatch_from_dyn(tcx: TyCtxt<'_>, impl_did: LocalDef | |||
use rustc_type_ir::TyKind::*; | |||
match (source.kind(), target.kind()) { | |||
(&Ref(r_a, _, mutbl_a), Ref(r_b, _, mutbl_b)) | |||
// FIXME(DefineOpaqueTypes): no test exercizes using `DefineOpaqueTypes::Yes` below. |
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.
// FIXME(DefineOpaqueTypes): no test exercizes using `DefineOpaqueTypes::Yes` below. | |
// FIXME(DefineOpaqueTypes): no test exercises using `DefineOpaqueTypes::Yes` below. |
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.
TIL
I saw that after starting and it's more complementary: it flipped uses to Some of the new |
And yeah, this work is kind-of underway in #117348. I actually slightly regret the approach suggested #116652. Seems a bit too much like busy work -- creating 36 commits that all just add a single Ideally we'd be working incrementally towards our goal of actually getting rid of |
Ok then let's close this. |
I noticed #116652 earlier, and this PR marks all the
DefineOpaqueTypes::No
use-sites that didn't make any test fail when flipped toDefineOpaqueTypes::Yes
(each into their dedicated commit as requested in that issue).r? @compiler-errors