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.

Sign up for GitHub

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

Jump to bottom

Infer type in irrefutable slice patterns with fixed length as array #113199

Merged
merged 11 commits into from
Aug 3, 2023
Merged

Infer type in irrefutable slice patterns with fixed length as array #113199

Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift click to select a range
2827aa9
try to infer array type from slice pattern
b-naber Jun 21, 2023
e18e376
add test, bless tests
b-naber Jun 21, 2023
b3413c6
only infer array type on irrefutable patterns
b-naber Jun 23, 2023
39c2785
add tests for refutable patterns
b-naber Jun 30, 2023
feb8bb1
combine arguments into PatInfo
b-naber Jul 3, 2023
843e2ee
add test for nested pattern
b-naber Jul 3, 2023
1c217b6
move els into DeclOrigin
b-naber Jul 5, 2023
65f92a5
address review
b-naber Jul 10, 2023
96af415
try to structurally resolve before try_resolve_slice_ty_to_array_ty
b-naber Jul 17, 2023
e1e7553
add more tests
b-naber Jul 17, 2023
5a9af37
address review
b-naber Jul 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
try to structurally resolve before try_resolve_slice_ty_to_array_ty
  • Loading branch information
@b-naber
b-naber committed Jul 17, 2023
commit 96af415476968d91847dde81c8ac907823b0def0
2 changes: 2 additions & 0 deletions compiler/rustc_hir_typeck/src/pat.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2114,6 2114,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
expected: Ty<'tcx>,
pat_info: PatInfo<'tcx, '_>,
) -> Ty<'tcx> {
let expected = self.try_structurally_resolve_type(span, expected);

// If the pattern is irrefutable and `expected` is an infer ty, we try to equate it
// to an array if the given pattern allows it. See issue #76342
if self.pat_is_irrefutable(pat_info.decl_origin) && expected.is_ty_var() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please move this if until after the structurally_resolve_type as it can otherwise miss info about the type of expected.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new tests previously failed because structurally_resolve_type returned an inference error.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, I guess we need try_structurally_resolve_type here 😅

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not convinced this is necessary. We only get into the demand_eqtype call when the pattern is irrefutable, how exactly would trying to structurally resolve the type help here? Afaict either there's a type annotation and we ignore all the logic introduced in this PR or there is none, in which case expected is just an unresolvable type variable afaict. Since we have to resolve the type after the demand_eqtype call anyway (since we still have to resolve the inner type after hinting that we're looking for [_; n]) I don't see why we want try_structurally_resolve_type first. Can you explain why that would be helpful/necessary?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we get into the demand_eqtype call when the pattern is irrefutable and the expected type is an inference variable. Whether the expected type is an inference variable can depend on structrally resolving it.

I can't imagine how you'd get to a point where it matters, but generally looking at the kind() of a type and then using structurally_resolve or shallow_resolve or whatever is always sus. Either the resolve is a noop, in which case it should be removed, or it can change the kind() of the type, in which case looking at the type before the resovle call is buggy.

Ah, nearly forgot: can you add tests for destructing assignments, i.e. what happens with the following

fn main() {
    let a: i32;
    let b;
    [a, b] = Default::default();
}

we should treat these as also being irrefutable and this example should compile i think

Expand Down