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

rustc should warn when the implementation of Display requires to display self #58035

Closed
rgiot opened this issue Jan 31, 2019 · 2 comments
Closed
Labels
A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@rgiot
Copy link

rgiot commented Jan 31, 2019

rustc should fail or warn when the implementation of the Display trait of a strcut requires to display it. Indeed there is a stack overflow as the code is recursively called.
rustc already detect recursion issues in other cases.
Such mistake can happen when using "{}" instead of "{:?}" for debug purpose.

This code:

use std::fmt;

struct Dummy {
}

impl fmt::Display for Dummy {
    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
        write!(f, "{}", self) // XXX Should fail or warn at compilation there
    }
}


impl Dummy {
    fn recursive(&self) {
        self.recursive(); /// XXX rustc warn there
    }
}

#[cfg(test)]
mod test {
    #[test]
    fn illustrate() {
        let dummy = crate::Dummy{};
        println!("{}", dummy);
    }
}

producde this compilation message and test error:

cargo test
   Compiling tmp v0.1.0 (/tmp)
warning: function cannot return without recursing
  --> src/lib.rs:13:5
   |
13 |     fn recursive(&self) {
   |     ^^^^^^^^^^^^^^^^^^^ cannot return without recursing
14 |         self.recursive();
   |         ---------------- recursive call site
   |
   = note: #[warn(unconditional_recursion)] on by default
   = help: a `loop` may express intention better if this is on purpose

warning: struct is never constructed: `Dummy`
 --> src/lib.rs:3:1
  |
3 | struct Dummy {
  | ^^^^^^^^^^^^
  |
  = note: #[warn(dead_code)] on by default

warning: method is never used: `recursive`
  --> src/lib.rs:13:5
   |
13 |     fn recursive(&self) {
   |     ^^^^^^^^^^^^^^^^^^^

warning: function cannot return without recursing
  --> src/lib.rs:13:5
   |
13 |     fn recursive(&self) {
   |     ^^^^^^^^^^^^^^^^^^^ cannot return without recursing
14 |         self.recursive();
   |         ---------------- recursive call site
   |
   = note: #[warn(unconditional_recursion)] on by default
   = help: a `loop` may express intention better if this is on purpose

warning: method is never used: `recursive`
  --> src/lib.rs:13:5
   |
13 |     fn recursive(&self) {
   |     ^^^^^^^^^^^^^^^^^^^
   |
   = note: #[warn(dead_code)] on by default

    Finished dev [unoptimized + debuginfo] target(s) in 0.23s
     Running target/debug/deps/tmp-dea596419d86ba6c

running 1 test

thread 'test::illustrate' has overflowed its stack
fatal runtime error: stack overflow

Although to have this crash is normal, the compiler should raise a warning exactly as it does for the recursive function.

With this version

impl fmt::Display for Dummy {
    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
        unimplemented!("{}", self) // XXX Should fail or warn at compilation there
    }
}

although the crash is slightly different:

thread panicked while processing panic. aborting.
error: process didn't exit successfully: `/tmp/target/debug/deps/tmp-dea596419d86ba6c` (signal: 4, SIGILL: illegal instruction)

compilation warning should be the same.

I'm writting this issue because I guess it can be detected by the compiler as Display (or Debug) belong to the language.

Meta

rustc --version --verbose
rustc 1.34.0-nightly (147311c5f 2019-01-30)
binary: rustc
commit-hash: 147311c5fc62537da8eb9c6f69536bec6719d534
commit-date: 2019-01-30
host: x86_64-unknown-linux-gnu
release: 1.34.0-nightly
LLVM version: 8.0
@jonas-schievink
Copy link
Contributor

One of the many instances of #57965

(the case for anything using std::fmt is more difficult since IIRC that heavily uses trait objects, though)

@jonas-schievink jonas-schievink added C-enhancement Category: An issue proposing an enhancement or a PR with one. A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jan 31, 2019
@estebank
Copy link
Contributor

Direct dupe of #45838, but there are multiple cases as pointed out by @jonas-schievink.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lints Area: Lints (warnings about flaws in source code) such as unused_mut. C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants