Bug 16211 – [REG 2.058] Cyclic dependencies broken again

Status
RESOLVED
Resolution
FIXED
Severity
regression
Priority
P1
Component
druntime
Product
D
Version
D2
Platform
All
OS
All
Creation time
2016-06-27T20:22:00Z
Last change time
2017-01-16T23:24:48Z
Keywords
pull
Assigned to
nobody
Creator
schveiguy

Comments

Comment #0 by schveiguy — 2016-06-27T20:22:58Z
The code that triggered https://issues.dlang.org/show_bug.cgi?id=4384 is once again compiling. Since a long time (2011). We currently have cycles in Phobos without realizing it because of this. We need to rethink how we do cycle detection. It was reworked in this PR: https://github.com/dlang/druntime/pull/114 However, one problem with this is a module with no ctors is once again marked as being "visited", so if it's needed 2+ times for a cycle, the cycle goes undetected. I don't want to necessarily go back to the alloca version, but perhaps we can remove the stack-based search and use recursion once again. I'm almost certain we will need at least 2x number of modules, but I'm not sure the limit. We also have to prevent infinite loops, which is why the original code was so weird. CC'ing Martin as he wrote the pull causing this.
Comment #1 by bugzilla — 2016-07-09T06:36:12Z
Comment #2 by github-bugzilla — 2016-08-21T18:00:37Z
Commits pushed to master at https://github.com/dlang/druntime https://github.com/dlang/druntime/commit/4b161d5771b458916cc1503969d592c89cc5768b Fixes issue 16211 - Fix cycle detection algorithm. Left the old algorithm there to make the diff uncomplicated. https://github.com/dlang/druntime/commit/0c912c4bd6aed8c0c01bb21eef0f7a418a773b6e Merge pull request #1602 from schveiguy/fixcycles Fix issue 16211 - Cyclic dependencies broken again
Comment #3 by github-bugzilla — 2016-12-23T13:54:37Z
Commit pushed to stable at https://github.com/dlang/druntime https://github.com/dlang/druntime/commit/5ad304a4323a80e581a5e8da61b9bd81205c9438 fixup for issue 16211 - Add deprecate feature for cycle detection as the default mode.
Comment #4 by github-bugzilla — 2016-12-23T18:25:58Z
Commit pushed to master at https://github.com/dlang/druntime https://github.com/dlang/druntime/commit/5ad304a4323a80e581a5e8da61b9bd81205c9438 fixup for issue 16211 - Add deprecate feature for cycle detection as the
Comment #5 by github-bugzilla — 2016-12-27T13:11:14Z
Commit pushed to scope at https://github.com/dlang/druntime https://github.com/dlang/druntime/commit/5ad304a4323a80e581a5e8da61b9bd81205c9438 fixup for issue 16211 - Add deprecate feature for cycle detection as the
Comment #6 by github-bugzilla — 2016-12-28T11:48:10Z
Commits pushed to stable at https://github.com/dlang/dlang.org https://github.com/dlang/dlang.org/commit/842b51bd1bc8dd6892f1d1122ed27d4f3ed20fce Add message about issue 16211 update It needs to be in the changelog for sure, but we had no bugzilla issue for it. https://github.com/dlang/dlang.org/commit/db6ae4de058973275514ecfefd478f6e2e766f53 Merge pull request #1538 from schveiguy/patch-1 Add message about issue 16211 update
Comment #7 by github-bugzilla — 2016-12-30T02:03:19Z
Comment #8 by github-bugzilla — 2017-01-16T23:24:23Z
Comment #9 by github-bugzilla — 2017-01-16T23:24:48Z
Commit pushed to newCTFE at https://github.com/dlang/druntime https://github.com/dlang/druntime/commit/5ad304a4323a80e581a5e8da61b9bd81205c9438 fixup for issue 16211 - Add deprecate feature for cycle detection as the