Enforce closure args + return type are WF

[compiler-errors]

I found this out when investigating #123461 (comment). Turns out we don't register WF obligations for closure args and return types, leading to the ICE.

I think this is a useful thing to check for, but I'd like to check what the fallout is. crater is complete.

Worst case, I think we should enforce this across an edition boundary (and possibly eventually migrate this for all editions) -- this should be super easy to do, since this is a check in HIR wfcheck, so it can be made edition dependent. I believe the regressions are manageable enough to not necessitate edition-specific behavior.

Fixes #123461

[rustbot]

r? @petrochenkov

rustbot has assigned @petrochenkov.
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

[compiler-errors]

oops, no assignee yet

@bors try

[bors]

⌛ Testing commit 04b9db9 with merge 18fb860...

[bors]

☀️ Try build successful - checks-actions
Build commit: 18fb860 (18fb860b8a8e7ca508746882328061d0fe352543)

[compiler-errors]

@craterbot check

[craterbot]

👌 Experiment pr-123531 created and queued.
🤖 Automatically detected try build 18fb860
🔍 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-123531 is now running

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

[craterbot]

🎉 Experiment pr-123531 is completed!
📊 4 regressed and 1 fixed (434664 total)
📰 Open the full report.

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

[compiler-errors]

Crater is clean, except for a couple of crates. Opened nkconnor/sharded#8 for the regression that affects one root crate and one dependent crate. Needs FCP.

r? types

[compiler-errors]

I'm gonna FCP this.

This PR makes sure that we check the WF of closure args and return types, since previously we were only checking the WF of the signature of item-like fns.

This is currently done in hir typeck, because we do inference which is necessary to actually determine the type of such args/returns, but that's an implementation detail and it could be moved elsewhere.

This check simply makes sure people write less sketchy code. I don't believe it fixes any soundness (though I wouldn't be surprised if that's not true); it just moves weird errors from the call site to the definition site.

@rfcbot fcp merge

[rfcbot]

Team member @compiler-errors has proposed to merge this. The next step is review by the rest of the tagged team members:

• @BoxyUwU
• @aliemjay
• @compiler-errors
• @jackh726
• @lcnr
• @nikomatsakis
• @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.

[rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

[compiler-errors]

@bors r=oli-obk

[bors]

📌 Commit 27abeea has been approved by oli-obk

It is now in the queue for this repository.

[compiler-errors]

@bors r-

[compiler-errors]

@bors r=oli-obk

[bors]

📌 Commit 870ed4b has been approved by oli-obk

It is now in the queue for this repository.

[bors]

⌛ Testing commit 870ed4b with merge 9e6c4fd...

[bors]

☀️ Test successful - checks-actions
Approved by: oli-obk
Pushing 9e6c4fd to master...

[rust-timer]

Finished benchmarking commit (9e6c4fd): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-3.0%	[-3.0%, -3.0%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-3.0%	[-3.0%, -3.0%]	1

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 670.846s -> 670.761s (-0.01%)
Artifact size: 316.00 MiB -> 315.95 MiB (-0.01%)

[apiraino]

@compiler-errors given the results of #125195, do you want this PR in relnotes? Thanks

[lcnr]

I think every accepted t-types FCP should be considered for relnotes by default. Will manually go through all of them and add them to the release notes once they are up, cc https://rust-lang.zulipchat.com/#narrow/stream/241545-t-release/topic/1.2E78/near/430858983

[apiraino]

good point @lcnr thanks!