D issues are now tracked on GitHub. This Bugzilla instance remains as a read-only archive.
Issue 16211 - [REG 2.058] Cyclic dependencies broken again
Summary: [REG 2.058] Cyclic dependencies broken again
Status: RESOLVED FIXED
Alias: None
Product: D
Classification: Unclassified
Component: druntime (show other issues)
Version: D2
Hardware: All All
: P1 regression
Assignee: No Owner
URL:
Keywords: pull
Depends on:
Blocks:
 
Reported: 2016-06-27 20:22 UTC by Steven Schveighoffer
Modified: 2017-01-16 23:24 UTC (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this issue.
Description Steven Schveighoffer 2016-06-27 20:22:58 UTC
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 Walter Bright 2016-07-09 06:36:12 UTC
https://github.com/dlang/druntime/pull/1602
Comment 2 github-bugzilla 2016-08-21 18:00:37 UTC
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 github-bugzilla 2016-12-23 13:54:37 UTC
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 github-bugzilla 2016-12-23 18:25:58 UTC
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 github-bugzilla 2016-12-27 13:11:14 UTC
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 github-bugzilla 2016-12-28 11:48:10 UTC
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 github-bugzilla 2016-12-30 02:03:19 UTC
Commits pushed to master at https://github.com/dlang/dlang.org

https://github.com/dlang/dlang.org/commit/842b51bd1bc8dd6892f1d1122ed27d4f3ed20fce
Add message about issue 16211 update

https://github.com/dlang/dlang.org/commit/db6ae4de058973275514ecfefd478f6e2e766f53
Merge pull request #1538 from schveiguy/patch-1
Comment 8 github-bugzilla 2017-01-16 23:24:23 UTC
Commits pushed to newCTFE at https://github.com/dlang/dlang.org

https://github.com/dlang/dlang.org/commit/842b51bd1bc8dd6892f1d1122ed27d4f3ed20fce
Add message about issue 16211 update

https://github.com/dlang/dlang.org/commit/db6ae4de058973275514ecfefd478f6e2e766f53
Merge pull request #1538 from schveiguy/patch-1
Comment 9 github-bugzilla 2017-01-16 23:24:48 UTC
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