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

new lint: Recursive Display impl #2691

Closed
oherrala opened this issue Apr 21, 2018 · 6 comments
Closed

new lint: Recursive Display impl #2691

oherrala opened this issue Apr 21, 2018 · 6 comments
Labels
C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages E-medium Call for participation: Medium difficulty level problem and requires some initial experience. L-correctness Lint: Belongs in the correctness lint group

Comments

@oherrala
Copy link

oherrala commented Apr 21, 2018

It's possible to implement Display trait in such a way which recursively calls itself until stack is exhausted. There's probably more variations how to encounter this bug. I provide one example.

Consider the following code:

use std::fmt::{Display, Formatter, Result};

struct Foo;

impl Display for Foo {
    fn fmt(&self, f: &mut Formatter) -> Result {
        write!(f, "{}", self)
    }
}

fn main() {
    println!("{}", Foo);
}

Compiles perfectly fine, but there's stack trace at run time:

$ cargo run -q

thread 'main' has overflowed its stack
fatal runtime error: stack overflow
Abort trap: 6

The bug is in line write!(f, "{}", self) which recursively calls itself by trying to use Display trait implementation for self.

@clarfonthey
Copy link

Huh, it's funny that this doesn't trigger the unconditional recursion lint upstream. I'd consider this to be a compiler bug.

@Manishearth
Copy link
Member

I suspect that's because the formatting machinery goes through some intermediate functions.

@clarfonthey
Copy link

Ah right, the lint only checks one layer of function calls, so fn a() { b() } fn b() { a() } doesn't lint.

@mimoo
Copy link

mimoo commented Oct 7, 2020

any update here :) ? This would be an awesome lint!

@flip1995 flip1995 added L-correctness Lint: Belongs in the correctness lint group E-medium Call for participation: Medium difficulty level problem and requires some initial experience. A-lint Area: New lints C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages and removed A-lint Area: New lints labels Oct 9, 2020
@flip1995
Copy link
Member

flip1995 commented Oct 9, 2020

There is the to_string_in_display lint, which already checks for one case. This lint could be enhanced to also check for things like using the formatting machinery inside the Display implementation.

@camsteffen camsteffen changed the title new lint: Display trait implementation can stack overflow new lint: Recursive Display impl Nov 2, 2021
bors added a commit that referenced this issue Feb 16, 2022
new lint: `recursive_format_impl`

The to_string_in_display lint is renamed to recursive_format_impl
A check is added for the use of self formatted with Display or Debug inside any format string in the same impl
The to_string_in_display check is kept as is - like in the format_in_format_args lint

This is my first contribution so please check it for better / more idiomatic checks + error messages. Note the format macro paths are shared with the `format_in_format_args` lint - maybe those could be moved to clippy utils too.

This relates to issues #2691 and #7830

------

changelog: Renamed `to_string_in_display` lint to [`recursive_format_impl`] with new check for any use of self as Display or Debug inside the same format trait impl.
@giraffate
Copy link
Contributor

It looks like that the recursive_format_impl covers this, so I'm closing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages E-medium Call for participation: Medium difficulty level problem and requires some initial experience. L-correctness Lint: Belongs in the correctness lint group
Projects
None yet
Development

No branches or pull requests

6 participants