Skip to content

Properly check that an expression might be the one returned #15115

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

samueltardieu
Copy link
Contributor

@samueltardieu samueltardieu commented Jun 23, 2025

The TyCtxt::hir_get_fn_id_for_return_block() function was too broad, as it will return positively even when given part of an expression that can be used as a return value. A new potential_return_of_enclosing_body() utility function has been made to represent the fact that an expression might be directly returned from its enclosing body.

changelog: [return_and_then]: prevent false positives in case of a partially used expression

Fixes #15111
Fixes #14927

Summary Notes

Managed by @rustbot—see help for details

@rustbot
Copy link
Collaborator

rustbot commented Jun 23, 2025

r? @llogiq

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

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jun 23, 2025
@samueltardieu
Copy link
Contributor Author

@rustbot label +beta-nominated.
@rustbot note Beta-nomination

The issue is beta-nominated because the extension of scope which causes the issue was introduced in #14783 which will be part of Rust 1.89.

@rustbot rustbot added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Jun 23, 2025
@y21
Copy link
Member

y21 commented Jun 23, 2025

I assume this also fixes #14927, right? I also commented in #14950 (comment) (PR that fixes the same issue) that it seemed odd to me that we're even processing expressions in these positions and that hir_get_fn_id_for_return_block should've filtered it out given the method name (or if it doesn't, we're probably misusing that function and should fix that part instead). So +1 for this approach

@samueltardieu
Copy link
Contributor Author

I had missed #14927, this fixes it indeed, and I've added the test case.

I was going to first investigate fixing hir_get_fn_id_for_return_block() in r-l/r, but I didn't want to wait for integration+next sync given that the issue has entered the beta branch. I've added this to my todo list though.

@samueltardieu samueltardieu marked this pull request as draft June 23, 2025 15:33
@samueltardieu samueltardieu force-pushed the issue-15111 branch 3 times, most recently from 8f896fc to 76ecbef Compare June 23, 2025 16:08
The `TyCtxt::hir_get_fn_id_for_return_block()` function was too broad,
as it will return positively even when given part of an expression that can
be used as a return value. A new `potential_return_of_enclosing_body()`
utility function has been made to represent the fact that an expression
might be directly returned from its enclosing body.
@samueltardieu samueltardieu marked this pull request as ready for review June 23, 2025 16:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta-nominated Nominated for backporting to the compiler in the beta channel. 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.

[return_and_then] False Positive on Nightly FP return_and_then
4 participants