Skip to content

Conversation

@flip1995
Copy link
Member

This simplifies the visitor code a bit and prevents checking expressions
multiple times. I still think this lint should be removed for now,
because its code isn't really tested.

Fixes #8689

NOTE: Before merging this, we should talk about removing and revisiting this lint. See my comment in #8689

changelog: prevent infinite recursion in [only_used_in_recursion]

This simplifies the visitor code a bit and prevents checking expressions
multiple times. I still think this lint should be removed for now,
because its code isn't really tested.
@rust-highfive
Copy link

r? @Manishearth

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Apr 12, 2022
@flip1995
Copy link
Member Author

r? @llogiq

@rust-highfive rust-highfive assigned llogiq and unassigned Manishearth Apr 12, 2022
return;
}
match ex.kind {
ExprKind::Array(exprs) | ExprKind::Tup(exprs) => {
Copy link
Member Author

Choose a reason for hiding this comment

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

An example for missing tests: This whole arm could be removed completely and the tests keep passing, even though this changes the semantics of the lint.

@llogiq
Copy link
Contributor

llogiq commented Apr 12, 2022

Thank you! That's what I would have done there, too.

@bors r+

@bors
Copy link
Contributor

bors commented Apr 12, 2022

📌 Commit 214fba7 has been approved by llogiq

@bors
Copy link
Contributor

bors commented Apr 12, 2022

⌛ Testing commit 214fba7 with merge f08a9c0...

@flip1995
Copy link
Member Author

NOTE: Before merging this, we should talk about removing and revisiting this lint. See my comment in #8689

That still stands. But let's talk about this on Zulip

@bors
Copy link
Contributor

bors commented Apr 12, 2022

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: llogiq
Pushing f08a9c0 to master...

@bors bors merged commit f08a9c0 into rust-lang:master Apr 12, 2022
bors added a commit that referenced this pull request May 3, 2022
Move only_used_in_recursion to nursery

r? `@llogiq`

I still think this is the right thing to do for this lint. See: #8782

I want to get this merged before the sync on Thursday, because I want to backport this and the last fix #8691 to beta.

changelog: Move [`only_used_in_recursion`] to nursery
@flip1995 flip1995 deleted the infinite_recursion_only_in_recursion branch May 12, 2022 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

Infinite loop in clippy::only_used_in_recursion

5 participants