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.

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

Prevent infinite (exponential) recursion in only_used_in_recursion #8691

Merged

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
}

fn visit_expr(&mut self, ex: &'tcx Expr<'tcx>) {
if !self.visited_exprs.insert(ex.hir_id) {
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
Collaborator

bors commented Apr 12, 2022

📌 Commit 214fba7 has been approved by llogiq

@bors
Copy link
Collaborator

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
Collaborator

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