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

Infinite recursion is not catched by the compiler. #70727

Open
wdanilo opened this issue Apr 3, 2020 · 5 comments
Open

Infinite recursion is not catched by the compiler. #70727

wdanilo opened this issue Apr 3, 2020 · 5 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

@wdanilo
Copy link

wdanilo commented Apr 3, 2020

Hi, consider this code:

#![deny(unconditional_recursion)]

struct Object {
    
}

impl Object {
    // RENAME HERE:
    fn foo(&self) -> f32 {
        7.0
    }
}

impl ToObject for Object {
    fn to_object(&self) -> &Object {
        self
    }
}

trait ToObject {
    fn to_object(&self) -> &Object;
}


trait ObjectOps : ToObject {
    fn foo(&self) -> f32 {
        self.to_object().foo()
    }
}

impl<T:ToObject> ObjectOps for T {}

fn main() {
    let x = Object{};
    println!("{}",x.foo());
}

It works fine. Let's rename foo to foo2 next to the comment "RENAME HERE". What happens is that this code has infinite recursion now and rustc doesn't report any error nor warning despite the usage of #![deny(unconditional_recursion)].

This is a serious problem, as a simple refactoring can cause something like that and it is very, very hard to debug, especially on WASM platform, where infinite recursion can just return a random number (it is UB). This problem could be solvable if I was able to write somehow MAGIC::Object::foo(self.to_object()) instead of self.to_object().foo() but in such a way, that the methods from ObjectOps would not be visible in MAGIC::Object. Currently, when refactoring such code we can get infinite recursion and spend hours debugging it :(

@wdanilo wdanilo added the C-bug Category: This is a bug. label Apr 3, 2020
@wdanilo wdanilo mentioned this issue Apr 3, 2020
3 tasks
@jumbatm
Copy link
Contributor

jumbatm commented Apr 3, 2020

Thanks for the report!

Very slightly trimmed test case:

#![deny(unconditional_recursion)]

struct Object;

trait ToObject {
    fn to_object(&self) -> &Object;
}

impl ToObject for Object {
    fn to_object(&self) -> &Object {
        self
    }
}

impl<T: ToObject> ObjectOps for T {}

trait ObjectOps : ToObject {
    fn foo(&self) -> f32 {
        self.to_object().foo()
    }
}

fn main() {
    let x = Object;
    println!("{}", x.foo());
}

The (false?) negative for unconditional_recursion essentially boils down to this case:

#![deny(unconditional_recursion)]
fn bar() {
    foo();
}

fn foo() {
    bar();
}

fn main() {
    foo();
}

ie, when the cycle in the callgraph is formed with at least one other function. This is because the lint currently only fires if a function directly calls itself, eg:

fn foo() {
    foo();
}

Relevant comment here:

//FIXME(#54444) rewrite this lint to use the dataflow framework
// Walk through this function (say `f`) looking to see if
// every possible path references itself, i.e., the function is
// called recursively unconditionally. This is done by trying
// to find a path from the entry node to the exit node that
// *doesn't* call `f` by traversing from the entry while
// pretending that calls of `f` are sinks (i.e., ignoring any
// exit edges from them).

@jonas-schievink
Copy link
Contributor

Part of #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. and removed C-bug Category: This is a bug. labels Apr 3, 2020
@ghost
Copy link

ghost commented Aug 18, 2020

Is this the same issue or should I open an extra issue?

struct Floaty {}

impl From<f64> for Floaty {
    fn from(value: f64) -> Floaty {
        value.into()
    }
}

fn main() {
    let _: Floaty = 1.0.into();
}

This does not generate any recursion warnings.

On musl any recursion segfaults, which I reported here: #75667. The above was my original case, util I realized that it segfaults for any recursions.

@jonas-schievink
Copy link
Contributor

@Etherealist Yeah, same issue

@jhg
Copy link

jhg commented Nov 14, 2020

Maybe this is also same issue:

use std::fmt;

#[derive(Copy, Clone)]
pub enum MyEnum {
    A,
    B,
}

impl AsRef<str> for MyEnum {
    fn as_ref(&self) -> &str {
        match self {
            Self::A => "a",
            Self::B => "b",
        }
    }
}

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

fn main() {
    println!("{}", MyEnum::A);
}

Maybe it is calling recursively to Display::fmt but the compiler does not detect it is calling to itself when it is a trait.

Edit: I see now #45838, maybe this is like that other issue more than like this issue.

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

4 participants