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

Fix check_infinite_loop (while_immutable_condition) by checking for break or return inside loop body #4730

Merged
merged 1 commit into from
Nov 23, 2019
Merged

Fix check_infinite_loop (while_immutable_condition) by checking for break or return inside loop body #4730

merged 1 commit into from
Nov 23, 2019

Conversation

yerke
Copy link
Contributor

@yerke yerke commented Oct 25, 2019 •
edited
Loading

changelog: Fix check_infinite_loop (while_immutable_condition) by checking for break or return inside loop body
fixes #4648

@yerke
Copy link
Contributor Author

yerke commented Oct 25, 2019

Next I will try to break this lint into 2 as suggested in #4648 (comment)

flip1995
flip1995 reviewed Oct 26, 2019
View reviewed changes
clippy_lints/src/loops.rs Outdated Show resolved Hide resolved
@flip1995 flip1995 added the S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) label Oct 28, 2019
yerke
yerke commented Nov 10, 2019
View reviewed changes
Copy link
Contributor Author

@yerke yerke left a comment

Choose a reason for hiding this comment

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

I didn't clean up, so don't worry about the mess right now. Please let me know if I am on the right path here. Thanks.

clippy_lints/src/loops.rs Outdated Show resolved Hide resolved
clippy_lints/src/loops.rs Outdated Show resolved Hide resolved
@yerke yerke requested a review from flip1995 November 10, 2019 08:57
flip1995
flip1995 reviewed Nov 12, 2019
View reviewed changes
clippy_lints/src/loops.rs Outdated Show resolved Hide resolved
flip1995
flip1995 reviewed Nov 12, 2019
View reviewed changes
clippy_lints/src/loops.rs Outdated Show resolved Hide resolved
flip1995
flip1995 requested changes Nov 12, 2019
View reviewed changes
Copy link
Member

@flip1995 flip1995 left a comment

Choose a reason for hiding this comment

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

This should be implemented as a Visitor. See comment above

@yerke yerke force-pushed the fix-check_infinite_loop branch 2 times, most recently from 45dc16f to 2342e6e Compare November 13, 2019 08:41
@yerke yerke requested a review from flip1995 November 13, 2019 08:42
@yerke yerke changed the title WIP: Fix check_infinite_loop (while_immutable_condition) by just updating the message Fix check_infinite_loop (while_immutable_condition) by checking for break or return inside loop body Nov 13, 2019
flip1995
flip1995 reviewed Nov 13, 2019
View reviewed changes
clippy_lints/src/loops.rs Outdated Show resolved Hide resolved
clippy_lints/src/loops.rs Show resolved Hide resolved
tests/ui/infinite_loop.rs Show resolved Hide resolved
@yerke yerke requested a review from flip1995 November 13, 2019 17:52
flip1995
flip1995 reviewed Nov 15, 2019
View reviewed changes
clippy_lints/src/loops.rs Outdated Show resolved Hide resolved
@yerke yerke requested a review from flip1995 November 22, 2019 07:35
@yerke
Copy link
Contributor Author

yerke commented Nov 22, 2019

@flip1995 Does it look like you wanted to see? Do I understand correctly that we probably don't need to split into another lint any more now?

BTW, I opened #4835 as I am hitting the same issue as Travis CI. But the tests passed before I merged the latest master into my branch.

flip1995
flip1995 approved these changes Nov 22, 2019
View reviewed changes
Copy link
Member

@flip1995 flip1995 left a comment

Choose a reason for hiding this comment

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

Does it look like you wanted to see?

Yes that's exactly what I wanted to see.

Do I understand correctly that we probably don't need to split into another lint any more now?

This PR is already a big improvement. But I'd rather still split the lint up in a correctness and a style lint.

BTW, I opened #4835 as I am hitting the same issue as Travis CI

I'll include this in #4825. Thanks for the report.

clippy_lints/src/loops.rs Outdated Show resolved Hide resolved
@yerke yerke force-pushed the fix-check_infinite_loop branch from 4265143 to 1cba0c9 Compare November 22, 2019 17:16
@yerke
Copy link
Contributor Author

yerke commented Nov 22, 2019 •
edited
Loading

@flip1995 Updated with your suggested changes, and made sure infinite_loop test still passes after #4825 is merged in.

@yerke yerke requested a review from flip1995 November 22, 2019 17:34
@flip1995
Copy link
Member

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Nov 23, 2019

📌 Commit 1cba0c9 has been approved by flip1995

flip1995 added a commit to flip1995/rust-clippy that referenced this pull request Nov 23, 2019
@flip1995
Rollup merge of rust-lang#4730 - yerke:fix-check_infinite_loop, r=fli…
a164a37 
…p1995

Fix check_infinite_loop (while_immutable_condition) by checking for break or return inside loop body

changelog: Fix check_infinite_loop (while_immutable_condition) by checking for break or return inside loop body
fixes rust-lang#4648
@flip1995 flip1995 mentioned this pull request Nov 23, 2019
Closed
bors added a commit that referenced this pull request Nov 23, 2019
@bors
Auto merge of #4838 - flip1995:rollup-defookd, r=flip1995
13d81bd 
Rollup of 6 pull requests

Successful merges:

 - #4730 (Fix check_infinite_loop (while_immutable_condition) by checking for break or return inside loop body)
 - #4764 (Allow casts from the result of `checked_abs` to unsigned)
 - #4766 (Fix false positive in derive_hash_xor_eq)
 - #4811 (Literal Representation Restructure)
 - #4820 (doc: fix the comment above the lint function)
 - #4830 (use more efficient code to generate repeated string)

Failed merges:

r? @ghost

changelog: none
flip1995 added a commit to flip1995/rust-clippy that referenced this pull request Nov 23, 2019
@flip1995
Rollup merge of rust-lang#4730 - yerke:fix-check_infinite_loop, r=fli…
016857d 
…p1995

Fix check_infinite_loop (while_immutable_condition) by checking for break or return inside loop body

changelog: Fix check_infinite_loop (while_immutable_condition) by checking for break or return inside loop body
fixes rust-lang#4648
@flip1995 flip1995 mentioned this pull request Nov 23, 2019
Merged
bors added a commit that referenced this pull request Nov 23, 2019
@bors
Auto merge of #4840 - flip1995:rollup-jqk3a3i, r=flip1995
eb8cd54 
Rollup of 5 pull requests

Successful merges:

 - #4730 (Fix check_infinite_loop (while_immutable_condition) by checking for break or return inside loop body)
 - #4766 (Fix false positive in derive_hash_xor_eq)
 - #4811 (Literal Representation Restructure)
 - #4820 (doc: fix the comment above the lint function)
 - #4830 (use more efficient code to generate repeated string)

Failed merges:

r? @ghost
@bors
Copy link
Contributor

bors commented Nov 23, 2019

⌛ Testing commit 1cba0c9 with merge d6c2fb7...

bors added a commit that referenced this pull request Nov 23, 2019
@bors
Auto merge of #4730 - yerke:fix-check_infinite_loop, r=flip1995
d6c2fb7 
Fix check_infinite_loop (while_immutable_condition) by checking for break or return inside loop body

changelog: Fix check_infinite_loop (while_immutable_condition) by checking for break or return inside loop body
fixes #4648
@flip1995
Copy link
Member

@bors retry queue prio

bors added a commit that referenced this pull request Nov 23, 2019
@bors
Auto merge of #4840 - flip1995:rollup-jqk3a3i, r=flip1995
cc35165 
Rollup of 5 pull requests

Successful merges:

 - #4730 (Fix check_infinite_loop (while_immutable_condition) by checking for break or return inside loop body)
 - #4766 (Fix false positive in derive_hash_xor_eq)
 - #4811 (Literal Representation Restructure)
 - #4820 (doc: fix the comment above the lint function)
 - #4830 (use more efficient code to generate repeated string)

Failed merges:

r? @ghost

changelog: none
@bors
Copy link
Contributor

bors commented Nov 23, 2019

⌛ Testing commit 1cba0c9 with merge cc35165...

@bors bors merged commit 1cba0c9 into rust-lang:master Nov 23, 2019
@yerke yerke deleted the fix-check_infinite_loop branch November 23, 2019 20:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@flip1995 flip1995 Awaiting requested review from flip1995

Assignees
No one assigned
Labels
S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

false positive on while_immutable_condition
3 participants
@yerke @flip1995 @bors