Ensure inferred let pattern types are well-formed

[Kivooeo]

View all comments

same as #157013 but with crater and fpc

r? lcnr

[rustbot]

r? @chenyukang

rustbot has assigned @chenyukang.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: compiler
• compiler expanded to 73 candidates
• Random selection from 19 candidates

[Kivooeo]

@bors try

[rust-bors]

☀️ Try build successful (CI)
Build commit: b152f55 (b152f5575a2ac7126ae0019e4e679abe9399213e, parent: 65407954098ca3c19f0d46092cb374b5d3e9dc3c)

[s7tya]

#150040

[Kivooeo]

@craterbot check

[craterbot]

👌 Experiment pr-157841 created and queued.
🤖 Automatically detected try build b152f55
🔍 You can check out the queue and this experiment's details.

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[craterbot]

🚧 Experiment pr-157841 is now running

ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

[craterbot]

🎉 Experiment pr-157841 is completed!
📊 2 regressed and 4 fixed (978411 total)
📊 4727 spurious results on the retry-regressed-list.txt, consider a retry¹ if this is a significant amount.
📰 Open the summary report.

⚠️ If you notice any spurious failure please add them to the denylist!
ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more

Footnotes

1. re-run the experiment with crates=https://crater-reports.s3.amazonaws.com/pr-157841/retry-regressed-list.txt ↩

[theemathas]

what the heck is this magic_import crate that pass_by_catastrophe depends on

[theemathas]

The two failures are spurious.

• The pass_by_catastrophe crate uses a proc macro from the magic_import crate. This macro automatically tries to guess, using some heuristics, what things should be imported. The heuristics failed, and the macro fell back to picking at random. It proceeded to incorrectly import the newly-stabilized Range type, instead of the old one.
• The records-in-rust crate has a build script that seemingly tries to write to the filesystem, which is (sometimes?) read-only on crater.

[lcnr]

Slightly adjusted from #157013 by @s7tya

This registers a well-formedness obligation for inferred let pattern types which never get fully initialized.

Previously, let PAT; without an explicit type annotation could infer a non-well-formed pattern input type, such as [str; 2] or (str, str). Some bindings inside the pattern can still have well-formed local types, for example ref x gives x: &str, so checking only the binding locals missed the enclosing array or tuple type. As a result, invalid unsized array/tuple pattern types were accepted and could later lead to layout ICEs.

The new check registers a WF obligation for the type of the whole pattern.

This is theoretically breaking existing incorrect code, but causes no crater regression.

@rfcbot fcp merge types

[rust-rfcbot]

@lcnr has proposed to merge this. The next step is review by the rest of the tagged team members:

• @BoxyUwU
• @jackh726
• @lcnr
• @oli-obk
• @spastorino

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[JonathanBrouwer]

💔 I suspect this PR failed tests as part of a rollup
@bors r-

After fixing the problem, consider running a try job for the failed job before re-approving.

Link to failure: #160863 (comment)

[rust-bors]

This pull request was unapproved.

This PR was contained in a rollup (#160863), which was unapproved.

View changes since this unapproval

[JonathanBrouwer]

@bors try jobs=x86_64-gnu-llvm-21-3
Double checking

[rust-bors]

💔 Test for 722aaf9 failed: CI. Failed job:

• try - x86_64-gnu-llvm-21-3 (web logs, enhanced plaintext logs)

[s7tya]

I think it's just a crash log that I reported, and it's already covered by the new tests. We can just remove tests/crashes/150040.rs. I'd be happy to do that, but I'm not sure whether I should open a new PR or have the maintainers modify my commit. I've opened #160874 just in case it's better to do the former.

[JonathanBrouwer]

I don't think it's illegal process-wise do to it in a separate PR, but I think it's nicer to do it in this one to keep the history in one place. I'll add the commit to this PR.

[rustbot]

This PR changes a file inside tests/crashes. If a crash was fixed, please move into the corresponding ui subdir and add 'Fixes #' to the PR description to autoclose the issue upon merge.

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[JonathanBrouwer]

@bors r=lcnr

[rust-bors]

📌 Commit 93bc4e8 has been approved by lcnr

It is now in the queue for this repository.