Skip to content
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

Closed
wants to merge 2 commits into from

Conversation

zhamlin
Copy link

@zhamlin zhamlin commented Oct 26, 2021

Part 1 of the plan defined here

@rust-highfive
Copy link
Collaborator

Some changes occured to the CTFE / Miri engine

cc @rust-lang/miri

@rust-highfive
Copy link
Collaborator

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.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 26, 2021
@rust-log-analyzer

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) {
Copy link
Contributor

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.

Copy link
Author

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.

Copy link
Contributor

@oli-obk oli-obk left a 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

compiler/rustc_borrowck/src/places_conflict.rs Outdated Show resolved Hide resolved
compiler/rustc_borrowck/src/type_check/mod.rs Outdated Show resolved Hide resolved
compiler/rustc_middle/src/mir/tcx.rs Outdated Show resolved Hide resolved
compiler/rustc_trait_selection/src/traits/select/mod.rs Outdated Show resolved Hide resolved
@zhamlin zhamlin force-pushed the placety/enum-variant-types branch from 04e53d5 to e35ffca Compare October 31, 2021 01:23
@rust-log-analyzer

This comment has been minimized.

@zhamlin
Copy link
Author

zhamlin commented Oct 31, 2021

For the unimplemented!, I can go ahead and update them to use recursion where possible.

@zhamlin zhamlin force-pushed the placety/enum-variant-types branch from e35ffca to a252860 Compare October 31, 2021 01:32
@rust-log-analyzer
Copy link
Collaborator

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
    Checking tracing-tree v0.1.9
    Checking rustdoc-json-types v0.1.0 (/checkout/src/rustdoc-json-types)
    Checking tera v1.10.0
    Checking rustdoc v0.0.0 (/checkout/src/librustdoc)
error[E0004]: non-exhaustive patterns: `Variant(_, _)` not covered
    --> src/librustdoc/clean/mod.rs:1396:15
     |
1396 |         match *ty.kind() {
     |               ^^^^^^^^^^ pattern `Variant(_, _)` not covered
    ::: /checkout/compiler/rustc_middle/src/ty/sty.rs:199:5
     |
     |
199  |     Variant(Ty<'tcx>, VariantIdx),
     |
     = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms
     = note: the matched value is of type `rustc_middle::ty::TyKind`


error[E0004]: non-exhaustive patterns: `Variant(_, _)` not covered
    |
    |
539 |         Some(match *self.cx.tcx.type_of(ty_id).kind() {
    |                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ pattern `Variant(_, _)` not covered
   ::: /checkout/compiler/rustc_middle/src/ty/sty.rs:199:5
    |
    |
199 |     Variant(Ty<'tcx>, VariantIdx),
    |
    = help: ensure that all possible cases are being handled, possibly by adding wildcards or more match arms
    = note: the matched value is of type `rustc_middle::ty::TyKind`

@zhamlin zhamlin closed this Nov 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants