Compiler doesn't warn on infinite recursion in trait's method

[DoumanAsh]

Example: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=4782b3bebc1e92369e5ff7b3c5b4a6e1

Visible on both stable and nightly.

Compiler warns on infinite recursion in case of regular method:

impl AsyncTokioTimer<RawTimer> {
    #[inline]
    ///Creates new instance
    pub fn new(_time: time::Duration) -> Self {
        Self::new(_time)
    }
}

Compiler doesn't issue the same warning in case of trait's method

impl<T: TimerFd> Timer for AsyncTokioTimer<T> {
    fn new(timeout: time::Duration) -> Self {
        debug_assert!(timeout.as_millis() <= u32::max_value().into());
        Self::new(timeout)
    }
}

I suspect it should?

[jonas-schievink]

It's because the debug_assert! might unwind instead of falling through to the recursion below

[SNCPlay42]

The Self::new in the trait impl there refers to the inherent new, not the one from the trait impl.

e.g. this code does not recurse infinitely: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=c31a36d86be2dfbb4a9702325d1fb36c

This isn't the case in your code because AsyncTokioTimer<T: TimerFd> is more general than AsyncTokioTimer<RawTimer> so the inherent impl isn't satisfied.

[DoumanAsh]

@jonas-schievink Ah you're correct, although to be honest I believe it should still warn, as otherwise you get abort at runtime without even knowing reason (since it is just stack overflow)
Stack overflow is not going to lead to panic, that's why I believe it would be better to warn if there is such recursive path

@SNCPlay42 in my case it does call itself, which leads to stack overflow otherwise I wouldn't crate issue

[jyn514]

Seems like a duplicate of #78474.

[jonas-schievink]

It does – but this issue actually invokes a diverging function, so the proposed fix for that issue in #78474 (comment) probably won't fix this one

[keiichiw]

I think the idea of #78474 (comment) can fix it because the return type of debug_assert! is () not !. In my understanding, we say "a function is diverging" only if the function never return a value.

[DoumanAsh]

debug_assert! should expand to optional panic which is !

[keiichiw]

I meant debug_assert! can seen as the same with the following function, whose return type is ().

fn debug_assert_func(flg : bool) -> () {
  if !flg {
    panic!()
  }
}

Ah, but, it's a macro not a function. So, the type-based approach in #78474 may not be applicable here...

[cbeck88]

I have another example:

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

It's because the debug_assert! might unwind instead of falling through to the recursion below

Maybe the lint could be like, if all non-error pathways (panic / return an error) recurse, so that every path is either infinite loop or an error, then that is flagged?

It looks that clang's lint is similar to rustc:

struct foo {
    int a;

    int f(int x) const {
        return f(x);
    }
};

int main() {
    foo{}.f(0);
}

That gives a warning:

$ clang++ -Wall main.cpp
main.cpp:4:24: warning: all paths through this function will call itself [-Winfinite-recursion]
    int f(int x) const {

but this version gives no warning:

struct foo {
    int a;

    int f(int x) const {
        if (x < 7) throw 42;
        return f(x);
    }
};

int main() {
    foo{}.f(0);
}

it would be really nice though if rustc would lint on some of these examples