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

unconditional recursion warning not displayed when passing &self #67932

Closed
yoshuawuyts opened this issue Jan 6, 2020 · 2 comments
Closed

unconditional recursion warning not displayed when passing &self #67932

yoshuawuyts opened this issue Jan 6, 2020 · 2 comments
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

@yoshuawuyts
Copy link
Member

Today I hit a segfault caused by stack overflow that wasn't caught by the compiler. This only occurred in debug mode, not release.

Current Behavior

The cause was the following piece of code, which passes the compiler without emitting any warnings:

use std::fmt::{self, Display};

struct Foo;

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

playground

As would be obvious to most people carefully reviewing this code, there's a clear case of unconditional recursion happening. We pass &self to the same impl over and over.

Expected behavior

A similar case is already caught by the compiler: passing self rather than &self to the same impl. This triggers the "unconditional recursion" warning. I would've expected the &self case to hit the same warning.

use std::fmt::{self, Display};

struct Foo;

impl Display for Foo {
    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
        Display::fmt(self, f)
    }
}
warning: function cannot return without recursing
 --> src/lib.rs:6:5
  |
6 |     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ cannot return without recursing
7 |         Display::fmt(self, f)
  |         --------------------- recursive call site
  |
  = note: `#[warn(unconditional_recursion)]` on by default
  = help: a `loop` may express intention better if this is on purpose

playground


It'd be nice if the &self case would also emit diagnostics when detecting infinite recursion. Seeing a segfault occur in your program can be pretty scary. Added to that it wasn't triggering for us in release mode, and searching for "debug stack overflow rust" yields some pretty poor results (because "stack overflow" is also the name of a popular website, heh).

This seems like a case where we could probably use better diagnostics. Thanks!

@jonas-schievink
Copy link
Contributor

The Display::fmt(&self, f) will actually call the Display impl for &Foo, which is the impl<T> Display for &T, which then calls into your impl again. This is something the lint does not currently handle (it only handles a function calling itself, not any other call graph cycles).

Adding this feature to the lint is currently tracked in #57965.

@jonas-schievink jonas-schievink added 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. labels Jan 6, 2020
@yoshuawuyts
Copy link
Member Author

Closing in favor of #57965

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

2 participants