-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Update PlaceTy to use ty::Variant #90330
Conversation
Some changes occured to the CTFE / Miri engine cc @rust-lang/miri |
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @matthewjasper (or someone else) soon. Please see the contribution instructions for more information. |
This comment has been minimized.
This comment has been minimized.
@@ -211,7 +211,7 @@ fn place_components_conflict<'tcx>( | |||
let proj_base = &borrow_place.projection[..access_place.projection.len() + i]; | |||
let base_ty = Place::ty_from(borrow_local, proj_base, body, tcx).ty; | |||
|
|||
match (elem, &base_ty.kind(), access) { | |||
match (elem, &base_ty.strip_variant_type().kind(), access) { |
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 should document each such use to explain why it doesn't matter. E.g. this case is because we only care about whether the type has a drop impl itself. Though we could just add a method to Ty that returns whether it has a drop impl and make that function work for both variant and adt.
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 went with the adding a method to Ty approach, seemed better than using strip_variant_type here.
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 didn't go through the unimplemented! calls yet
04e53d5
to
e35ffca
Compare
This comment has been minimized.
This comment has been minimized.
For the unimplemented!, I can go ahead and update them to use recursion where possible. |
e35ffca
to
a252860
Compare
The job Click to see the possible cause of the failure (guessed by this bot)
|
Part 1 of the plan defined here