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

Use forward traversal for unconditional recursion lint #70973

Merged
merged 1 commit into from
Apr 10, 2020

Conversation

ecstatic-morse
Copy link
Contributor

@ecstatic-morse ecstatic-morse commented Apr 10, 2020

While reviewing #70822, I noted that #54444 could be solved without requiring the predecessor graph and without allocating a Vec<Span> for every basic block. The unconditional recursion lint is not a performance bottleneck however, so I approved #70822 as it was.

Nevertheless, I wanted to try implementing my idea using TriColorDepthFirstSearch, which is a DFS that can differentiate between forward/tree edges and backward ones. I found this approach more straightforward than the existing one, so I'm opening this PR to see if it is desirable.

The pass is now just a DFS across the control-flow graph. We ignore false edges and false unwinds, as well as the successors of recursive calls, just like existing pass does. If we see a back-edge (loop) or a terminator that would cause us to yield control-flow back to the caller (Return, Resume, etc.), we know that the function does not unconditionally recurse.

r? @jonas-schievink

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 10, 2020
@jonas-schievink
Copy link
Contributor

Neat, that does look simpler!

@bors r+

@bors
Copy link
Contributor

bors commented Apr 10, 2020

📌 Commit 0fc0f34 has been approved by jonas-schievink

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 10, 2020
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 10, 2020
Rollup of 9 pull requests

Successful merges:

 - rust-lang#69745 (Use `PredicateObligation`s instead of `Predicate`s)
 - rust-lang#70938 (Add ThreadSanitizer test case)
 - rust-lang#70973 (Use forward traversal for unconditional recursion lint)
 - rust-lang#70978 (compiletest: let config flags overwrite -A unused)
 - rust-lang#70979 (Follow up on BTreeMap comments)
 - rust-lang#70981 (Rearrange BTreeMap::into_iter to match range_mut.)
 - rust-lang#70985 (Clean up E0512 explanation)
 - rust-lang#70988 (Setup the `@rustbot prioritize` command)
 - rust-lang#70991 (fix rustc-dev-guide's url in src/librustc_codegen_ssa)

Failed merges:

r? @ghost
@bors bors merged commit 0d89287 into rust-lang:master Apr 10, 2020
@ecstatic-morse ecstatic-morse deleted the recursion-lint branch October 6, 2020 01:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants