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

Check whether the next_node is else-less if in get_return_block #124917

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Check whether the next_node is else-less if in get_return_block #124917

wants to merge 3 commits into from
+18 −0

Conversation

cardigan1008
Copy link

@cardigan1008 cardigan1008 commented May 9, 2024 •
edited

Fix #124819

@rustbot
Copy link
Collaborator

rustbot commented May 9, 2024

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @pnkfelix (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels May 9, 2024
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@cardigan1008 cardigan1008 mentioned this pull request May 9, 2024
Open
@cardigan1008
fix: Check whether next_node is else-less if in get_return_block
62318b3 
Fix rust-lang#124819, where a if-less block causes a wrong output. It is
caused by get_return_block in get_fn_decl. In get_return_block,
when a else-less if expression is the tail expression, the check
for next_node will keep iterating. So it is necessary to make a
early return in the check.
@cardigan1008 cardigan1008 marked this pull request as ready for review May 9, 2024 16:37
@cardigan1008 cardigan1008 changed the title fix: Add if to check whether the previous node is Block Check whether the next_node is else-less if in get_return_block May 9, 2024
@pnkfelix
Copy link
Member

can you add a unit test illustrating the change?

(If you need help with what steps to use to add a test, feel free to ask; I'd probably start by reading this https://rustc-dev-guide.rust-lang.org/tests/ui.html )

@pnkfelix
Copy link
Member

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 14, 2024
@cardigan1008
Copy link
Author

Hi @pnkfelix I have updated the unit test following the instruction. If there's any mistake, feel free to ask me to change. Thanks!

@cardigan1008
Copy link
Author

@rustbot review

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@pnkfelix pnkfelix

Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Incorrect span label when non-() else-less if is tail expression in -> () function
4 participants
@cardigan1008 @rustbot @rust-log-analyzer @pnkfelix