Add a lint unconditional_recursion to detect unconditional recursion.
#20373
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
r? @luqmana (rust_highfive has picked a reviewer for you, use r? to override) |
|
This needs a rebase |
|
@luqmana, speaking prescriptively, "recurs" is actually more correct than "recurses", e.g. http://english.stackexchange.com/questions/163446/does-a-recursive-procedure-recur ; I'm happy to change it if people prefer the "recurse" version. |
|
@frewsxcv (Thanks! I'm actually leaving it unrebased so it can't be r+'d and landed before I resolve the questions described above.) |
This allows one to look at an `ExprMethodCall` `foo.bar()` where `bar`
is a method in some trait and (sometimes) extract the `impl` that `bar`
is defined in, e.g.
trait Foo {
fn bar(&self);
}
impl Foo for uint { // <A>
fn bar(&self) {}
}
fn main() {
1u.bar(); // impl_def_id == Some(<A>)
}
This definitely doesn't handle all cases, but is correct when it is
known, meaning it should only be used for certain linting/heuristic
purposes; no safety analysis.
|
Updated after discussions with @nikomatsakis. (Also, this found a bug in the compiler. 😄) I measured the total time required to construct all the CFGs in each crate (sorted from slowest to fastest), I think it's safe to say that I do not need to worry about the performance of this at the moment since even constructing the graphs for all functions in librustc is only 50 milliseconds.
(Collected with this diff + some emacs macros etc.) |
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.
This was detected by the unconditional_recursion lint.
bors
added a commit
that referenced
this pull request
Jan 25, 2015
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.
| }); | ||
| } | ||
|
|
||
| // check the number of sell calls because a function that |
huonw
changed the title
unconditional_self_calls to detect unconditional recursion.unconditional_recursion to detect unconditional recursion.
ghost
mentioned this pull request
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
E.g.
fn foo() { foo() }, or, more subtlelyThe compiler will warn and point out the points where recursion occurs,
if it determines that the function cannot return without calling itself.
Closes #17899.