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

Add a cycle detector for generic `Graph`s and `mir::Body`s #64622

Merged
merged 2 commits into from Sep 25, 2019

Conversation

@ecstatic-morse
Copy link
Contributor

commented Sep 19, 2019

Cycle detection is one way to differentiate the upcoming const_loop feature flag (#52000) from the const_if_match one (#49146). It would be possible to use the existing implementation of strongly-connected components for this but less efficient.

The "tri-color" terminology is common in introductory data structures and algorithms courses: black nodes are settled, grey nodes are visited, and white nodes have no state. This particular implementation is iterative and uses a well-known technique where "node settled" events are kept on the stack alongside nodes to visit. When a settled event is popped, we know that all successors of that node have been visited and themselves settled. If we encounter a successor node that has been visited (is on the stack) but not yet settled, we have found a cycle.

r? @eddyb

@ecstatic-morse

This comment has been minimized.

Copy link
Contributor Author

commented Sep 19, 2019

This probably doesn't need to be reviewed by @eddyb (I r?-ed out of habit).

@ecstatic-morse

This comment has been minimized.

Copy link
Contributor Author

commented Sep 19, 2019

Maybe r? @Centril

@rust-highfive rust-highfive assigned Centril and unassigned eddyb Sep 19, 2019
@Centril

This comment has been minimized.

Copy link
Member

commented Sep 19, 2019

Maybe r? @Centril

Wouldn't even know where to begin. =P

r? @pnkfelix

@Centril Centril assigned pnkfelix and unassigned Centril Sep 19, 2019
@ecstatic-morse ecstatic-morse force-pushed the ecstatic-morse:cycle-detector branch from b6a63b9 to c3aeef7 Sep 19, 2019
@oli-obk

This comment has been minimized.

Copy link
Contributor

commented Sep 23, 2019

r? @oli-obk

Some more docs would indeed be great.

@rust-highfive rust-highfive assigned oli-obk and unassigned pnkfelix Sep 23, 2019
@ecstatic-morse ecstatic-morse force-pushed the ecstatic-morse:cycle-detector branch from c3aeef7 to 3cfe61a Sep 23, 2019
@ecstatic-morse ecstatic-morse force-pushed the ecstatic-morse:cycle-detector branch from 3cfe61a to c9e4816 Sep 23, 2019
@ecstatic-morse

This comment has been minimized.

Copy link
Contributor Author

commented Sep 23, 2019

I added some more docs comparing this depth-first search to the one in Introduction to Algorithms. Hopefully it's easier to follow now.

@oli-obk

This comment has been minimized.

Copy link
Contributor

commented Sep 24, 2019

@bors r+

Thanks, that helps

@bors

This comment has been minimized.

Copy link
Contributor

commented Sep 24, 2019

📌 Commit c9e4816 has been approved by oli-obk

Centril added a commit to Centril/rust that referenced this pull request Sep 24, 2019
…i-obk

Add a cycle detector for generic `Graph`s and `mir::Body`s

Cycle detection is one way to differentiate the upcoming `const_loop` feature flag (rust-lang#52000) from the `const_if_match` one (rust-lang#49146). It would be possible to use the existing implementation of strongly-connected components for this but less efficient.

The ["tri-color" terminology](http://www.cs.cornell.edu/courses/cs2112/2012sp/lectures/lec24/lec24-12sp.html) is common in introductory data structures and algorithms courses: black nodes are settled, grey nodes are visited, and white nodes have no state. This particular implementation is iterative and uses a well-known technique where "node settled" events are kept on the stack alongside nodes to visit. When a settled event is popped, we know that all successors of that node have been visited and themselves settled. If we encounter a successor node that has been visited (is on the stack) but not yet settled, we have found a cycle.

r? @eddyb
bors added a commit that referenced this pull request Sep 24, 2019
Rollup of 10 pull requests

Successful merges:

 - #63356 (Issue#63183: Add fs::read_dir() and ReadDir warning about iterator order + example)
 - #64296 (Document the unstable iter_order_by library feature)
 - #64428 (Error explanation e0524)
 - #64481 (A more explanatory thread local storage panic message)
 - #64546 (Bugfix/rfc 2451 rerebalance tests)
 - #64622 (Add a cycle detector for generic `Graph`s and `mir::Body`s)
 - #64689 (Refactor macro by example)
 - #64702 (Remove unused dependencies)
 - #64717 (update mem::discriminant test to use assert_eq and assert_ne over comparison operators)
 - #64725 (fix one typo)

Failed merges:

r? @ghost
bors added a commit that referenced this pull request Sep 24, 2019
Rollup of 10 pull requests

Successful merges:

 - #63356 (Issue#63183: Add fs::read_dir() and ReadDir warning about iterator order + example)
 - #64296 (Document the unstable iter_order_by library feature)
 - #64428 (Error explanation e0524)
 - #64481 (A more explanatory thread local storage panic message)
 - #64546 (Bugfix/rfc 2451 rerebalance tests)
 - #64622 (Add a cycle detector for generic `Graph`s and `mir::Body`s)
 - #64689 (Refactor macro by example)
 - #64702 (Remove unused dependencies)
 - #64717 (update mem::discriminant test to use assert_eq and assert_ne over comparison operators)
 - #64725 (fix one typo)

Failed merges:

r? @ghost
Centril added a commit to Centril/rust that referenced this pull request Sep 24, 2019
…i-obk

Add a cycle detector for generic `Graph`s and `mir::Body`s

Cycle detection is one way to differentiate the upcoming `const_loop` feature flag (rust-lang#52000) from the `const_if_match` one (rust-lang#49146). It would be possible to use the existing implementation of strongly-connected components for this but less efficient.

The ["tri-color" terminology](http://www.cs.cornell.edu/courses/cs2112/2012sp/lectures/lec24/lec24-12sp.html) is common in introductory data structures and algorithms courses: black nodes are settled, grey nodes are visited, and white nodes have no state. This particular implementation is iterative and uses a well-known technique where "node settled" events are kept on the stack alongside nodes to visit. When a settled event is popped, we know that all successors of that node have been visited and themselves settled. If we encounter a successor node that has been visited (is on the stack) but not yet settled, we have found a cycle.

r? @eddyb
bors added a commit that referenced this pull request Sep 24, 2019
Rollup of 16 pull requests

Successful merges:

 - #63356 (Issue#63183: Add fs::read_dir() and ReadDir warning about iterator order + example)
 - #63934 (Fix coherence checking for impl trait in type aliases)
 - #64016 (Streamline `Compiler`)
 - #64296 (Document the unstable iter_order_by library feature)
 - #64443 (rustdoc: general cleanup)
 - #64622 (Add a cycle detector for generic `Graph`s and `mir::Body`s)
 - #64689 (Refactor macro by example)
 - #64698 (Recover on `const X = 42;` and infer type + Error Stash API)
 - #64702 (Remove unused dependencies)
 - #64717 (update mem::discriminant test to use assert_eq and assert_ne over comparison operators)
 - #64720 ( remove rtp.rs, and move rtpSpawn and RTP_ID_ERROR to libc)
 - #64721 (Fixed issue from #64447)
 - #64725 (fix one typo)
 - #64737 (fix several issues in String docs)
 - #64742 (relnotes: make compatibility section more sterile and fix rustc version)
 - #64748 (Fix #64744. Account for the Zero sub-pattern case.)

Failed merges:

r? @ghost
@bors bors merged commit c9e4816 into rust-lang:master Sep 25, 2019
4 checks passed
4 checks passed
pr Build #20190923.34 succeeded
Details
pr (Linux mingw-check) Linux mingw-check succeeded
Details
pr (Linux x86_64-gnu-llvm-6.0) Linux x86_64-gnu-llvm-6.0 succeeded
Details
pr (LinuxTools) LinuxTools succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.