-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Add lint for direct recursive self-call of function #17899
Comments
I think an infinite loop written with tail recursion will be rare enough that that is not a problem. |
Personally I've hit a lot of infinite recursions caused by this, a lint would help a lot, especially since it's not really obvious how to fix the problem. |
Me too! |
I have this mostly implemented, except for some edge cases with |
Specifically, given trait Foo {
fn bar(&self); // A
}
impl Foo for uint {
fn bar(&self) { // B
self.bar(); // C
}
} I want to check if the method call C is calling method B, that is, obtain the DefId of the method called in the However, I can only work out how to obtain the DefId of A (via Advice, anyone? |
Niko gave me a lot of advice on IRC. |
E.g. `fn foo() { foo() }`, or, more subtlely impl Foo for Box<Foo+'static> { fn bar(&self) { self.bar(); } } The compiler will warn and point out the points where recursion occurs, if it determines that the function cannot return without calling itself. Closes rust-lang#17899.
E.g. `fn foo() { foo() }`, or, more subtlely impl Foo for Box<Foo+'static> { fn bar(&self) { self.bar(); } } The compiler will warn and point out the points where recursion occurs, if it determines that the function cannot return without calling itself. Closes #17899. --- This is highly non-perfect, in particular, my wording above is quite precise, and I have some unresolved questions: This currently will warn about ```rust fn foo() { if bar { loop {} } foo() } ``` even though `foo` may never be called (i.e. our apparent "unconditional" recursion is actually conditional). I don't know if we should handle this case, and ones like it with `panic!()` instead of `loop` (or anything else that "returns" `!`). However, strictly speaking, it seems to me that changing the above to not warn will require changing ```rust fn foo() { while bar {} foo() } ``` to also not warn since it could be that the `while` is an infinite loop and doesn't ever hit the `foo`. I'm inclined to think we let these cases warn since true edge cases like the first one seem rare, and if they do occur they seem like they would usually be typos in the function call. (I could imagine someone accidentally having code like `fn foo() { assert!(bar()); foo() /* meant to be boo() */ }` which won't warn if the `loop` case is "fixed".) (Part of the reason this is unresolved is wanting feedback, part of the reason is I couldn't devise a strategy that worked in all cases.) --- The name `unconditional_self_calls` is kinda clunky; and this reconstructs the CFG for each function that is linted which may or may not be very expensive, I don't know.
With trait objects it is not so rare to have code like
One might first write
self.bar(x)
, but this will be calling that same bar, via sinceBox<Foo>
has a "real"bar
method itself. The correct procedure is(**self).bar(x)
. This would helped by detecting the most obvious cases of a function calling itself in a nonterminating way. The most obvious way that is likely to have low risk of false positives is detecting if the CFG of a function reaches a self-call unconditionally. e.g.Notably, it does not matter if the arguments to the self call are different, since the call will always occur, and it also does not matter if the self-call is in tail position.
There is the possibility that someone is using infinite recursion to achieve an infinite loop, it doesn't seem crazy to recommend that they use
loop
instead.A more conservative alternative would be just detecting the method call in the first example, that is, a function that consists of a single expression that is a self-call.
The text was updated successfully, but these errors were encountered: