Skip to content

Eternal lint on a recursive tree #13544

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

Closed
cyypherus opened this issue Oct 13, 2024 · 7 comments · Fixed by #14641
Closed

Eternal lint on a recursive tree #13544

cyypherus opened this issue Oct 13, 2024 · 7 comments · Fixed by #14641
Labels
C-bug Category: Clippy is not doing the correct thing

Comments

@cyypherus
Copy link

cyypherus commented Oct 13, 2024

Summary

This code causes clippy to hang & never complete linting.

use std::mem::ManuallyDrop;
use std::ops::Deref;
use std::ops::DerefMut;

trait Scopable: Sized {
    type SubType: Scopable;
}

struct Subtree<T: Scopable>(ManuallyDrop<Box<Tree<T::SubType>>>);

impl<T: Scopable> Drop for Subtree<T> {
    fn drop(&mut self) {
        // SAFETY: The field cannot be used after we drop
        unsafe { ManuallyDrop::drop(&mut self.0) }
    }
}

impl<T: Scopable> Deref for Subtree<T> {
    type Target = Tree<T::SubType>;
    fn deref(&self) -> &Self::Target {
        &self.0
    }
}

impl<T: Scopable> DerefMut for Subtree<T> {
    fn deref_mut(&mut self) -> &mut Self::Target {
        &mut self.0
    }
}

enum Tree<T: Scopable> {
    Group(Vec<Tree<T>>),
    Subtree(Subtree<T>),
    Leaf(T),
}

impl<T: Scopable> Tree<T> {
    fn foo(self) -> Self {
        self
    }
}

fn main() {}

Reproducer

I tried this code:(See above)

I expected to see this happen: Clippy finish linting within a reasonable time period

Instead, this happened: Clippy doesn't finish linting & hangs, eating 90% cpu

Version

rustc 1.83.0-nightly (1bc403daa 2024-10-11)
binary: rustc
commit-hash: 1bc403daadbebb553ccc211a0a8eebb73989665f
commit-date: 2024-10-11
host: aarch64-apple-darwin
release: 1.83.0-nightly
LLVM version: 19.1.1

Additional Labels

No response

@cyypherus cyypherus added the C-bug Category: Clippy is not doing the correct thing label Oct 13, 2024
@y21
Copy link
Member

y21 commented Oct 13, 2024

Looks like a hang in the significant_drop_tightening lint. Reduced a bit further:

use std::marker::PhantomData;

trait Trait {
    type Assoc: Trait;
}
struct S<T: Trait>(*const S<T::Assoc>, PhantomData<T>);

fn f<T: Trait>(x: &mut S<T>) {
    &mut x.0;
}

fn main() {}

We keep recursing into the first field of the struct and always miss the type cache because each level of nesting adds another projection (S<T::Assoc> -> S<T::Assoc::Assoc> -> S<T::Assoc::Assoc::Assoc> ...).

@c410-f3r
Copy link
Contributor

c410-f3r commented Oct 13, 2024

Sigh... There are many errors related to significant_drop_tightening. I hope to get back to working on it in the coming weeks.

@theemathas
Copy link
Contributor

theemathas commented Apr 17, 2025

The below code causes cargo clippy to hang in rust version 1.86.0, but cargo clippy works fine for version 1.87.0-beta.3.

trait Bar {
    type Element: Bar;
}

enum Foo<T: Bar> {
    A(*const Foo<T::Element>),
    B(T),
}

(In addition, it causes rustdoc to hang. See rust-lang/rust#139964)

@theemathas
Copy link
Contributor

For y21's code:

  • cargo clippy still hangs on stable rust 1.86.0
  • cargo clippy works fine on beta rust 1.87.0-beta.3, but adding #![warn(clippy::significant_drop_tightening)] causes it to hang again

Adding #![warn(clippy::significant_drop_tightening)] appears to have no effect on my code.

@y21
Copy link
Member

y21 commented Apr 17, 2025

For the record, the code by @theemathas above triggered a bug in an unrelated util function (used in the large_enum_variants lint) and was fixed by #13833, where a general recursion limit was added to that function.

The code in the OP (and my reproducer) triggers a different bug in significant_drop_tightening, which is why #![warn(significant_drop_tightening)] is needed for specifically only those two (the lint is in nursery so would normally be filtered out and the buggy code doesn't run).

I imagine this issue could probably be fixed in the same way as the other issue was fixed (just bail if we recurse too much in this function, which is where the stack overflow/hang happens, and conservatively return false)

@samueltardieu
Copy link
Contributor

samueltardieu commented Apr 17, 2025

I imagine this issue could probably be fixed in the same way as the other issue was fixed (just bail if we recurse too much in this function, which is where the stack overflow/hang happens, and conservatively return false)

Indeed, done this way in #14641 a few minutes ago while you were writing this message, and assigned to you for review 😃

@samueltardieu
Copy link
Contributor

For the record, the code by @theemathas above triggered a bug in an unrelated util function (used in the large_enum_variants lint) and was fixed by #13833, where a general recursion limit was added to that function.

And the PR missed the Clippy sync by one day, which made it miss master→1.86, which is why it will be present only in 1.87.

github-merge-queue bot pushed a commit that referenced this issue Apr 17, 2025
Limit the recursion depth, as each level of nesting adds another deeper
projection.

There might be a more complex way of handling the problem, but infinite
recursions are bad, and don't allow Clippy to terminate properly.

changelog: [`significant_drop_tightening`]: do not recurse forever when
checking for attribute on type or its constituent

Fixes #13544

@rustbot label +L-nursery
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants