Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(618)

Issue 9663047: code review 9663047: cmd/gc: fix detection of initialization loop. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 years, 1 month ago by remyoudompheng
Modified:
8 years, 10 months ago
Reviewers:
DMorsing
CC:
golang-dev, DMorsing, rsc
Visibility:
Public.

Description

cmd/gc: fix detection of initialization loop. The compiler computes initialization order by finding a spanning tree between a package's global variables. But it does so by walking both variables and functions and stops detecting cycles between variables when they mix with a cycle of mutually recursive functions. Fixes issue 4847.

Patch Set 1 #

Patch Set 2 : diff -r ca166884c853 https://go.googlecode.com/hg/ #

Patch Set 3 : diff -r ca166884c853 https://go.googlecode.com/hg/ #

Patch Set 4 : diff -r ca166884c853 https://go.googlecode.com/hg/ #

Patch Set 5 : diff -r ca166884c853 https://go.googlecode.com/hg/ #

Patch Set 6 : diff -r 3833ddddde2b https://go.googlecode.com/hg/ #

Total comments: 1

Patch Set 7 : diff -r 80771ad49cb6 https://go.googlecode.com/hg/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+73 lines, -29 lines) Patch
M src/cmd/gc/sinit.c View 1 2 3 4 5 6 5 chunks +49 lines, -29 lines 0 comments Download
A test/fixedbugs/issue4847.go View 1 2 3 4 1 chunk +24 lines, -0 lines 0 comments Download

Messages

Total messages: 16
remyoudompheng
Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com), I'd like you to review this change to https://go.googlecode.com/hg/
9 years, 1 month ago (2013-05-27 06:10:04 UTC) #1
remyoudompheng
The error message is a bit ugly but can be improved separately: issue4847.go:20: initialization loop: ...
9 years, 1 month ago (2013-05-27 06:11:32 UTC) #2
DMorsing
LGTM. CL description is a bit weird near the end.
9 years, 1 month ago (2013-05-27 07:29:40 UTC) #3
remyoudompheng
On 2013/05/27 07:29:40, DMorsing wrote: > LGTM. > > CL description is a bit weird ...
9 years, 1 month ago (2013-05-29 21:44:26 UTC) #4
remyoudompheng
On 2013/05/29 21:44:26, remyoudompheng wrote: > I'll try to make the description readable. Done.
9 years, 1 month ago (2013-05-30 06:10:49 UTC) #5
remyoudompheng
Please take another look. I am a bit worried by the quadratic complexity. By compiling ...
9 years, 1 month ago (2013-05-30 06:22:43 UTC) #6
DMorsing
On 2013/05/30 06:22:43, remyoudompheng wrote: > Please take another look. > > I am a ...
9 years, 1 month ago (2013-05-30 06:28:46 UTC) #7
remyoudompheng
On 2013/05/30 06:28:46, DMorsing wrote: > On 2013/05/30 06:22:43, remyoudompheng wrote: > > Please take ...
9 years, 1 month ago (2013-06-01 08:43:57 UTC) #8
rsc
I would like to avoid quadratic algorithms in general. I don't understand this well enough ...
9 years ago (2013-06-10 20:44:22 UTC) #9
remyoudompheng
On 2013/06/10 20:44:22, rsc wrote: > I would like to avoid quadratic algorithms in general. ...
9 years ago (2013-06-11 06:01:08 UTC) #10
rsc
Thanks for the description - I understand the problem now. Isn't there some sub-quadratic way ...
9 years ago (2013-06-21 19:43:35 UTC) #11
remyoudompheng
Hello golang-dev@googlegroups.com, daniel.morsing@gmail.com, rsc@golang.org (cc: golang-dev@googlegroups.com), Please take another look.
8 years, 10 months ago (2013-08-14 22:03:17 UTC) #12
remyoudompheng
On 2013/08/14 22:03:17, remyoudompheng wrote: > Hello mailto:golang-dev@googlegroups.com, mailto:daniel.morsing@gmail.com, mailto:rsc@golang.org (cc: > mailto:golang-dev@googlegroups.com), > > ...
8 years, 10 months ago (2013-08-14 22:05:04 UTC) #13
remyoudompheng
ping?
8 years, 10 months ago (2013-08-28 06:48:06 UTC) #14
DMorsing
LGTM again. https://codereview.appspot.com/9663047/diff/23001/src/cmd/gc/sinit.c File src/cmd/gc/sinit.c (right): https://codereview.appspot.com/9663047/diff/23001/src/cmd/gc/sinit.c#newcode89 src/cmd/gc/sinit.c:89: loop: This label can be misread as ...
8 years, 10 months ago (2013-08-28 07:31:59 UTC) #15
remyoudompheng
8 years, 10 months ago (2013-08-29 08:16:20 UTC) #16
*** Submitted as https://code.google.com/p/go/source/detail?r=f2e954f463e0 ***

cmd/gc: fix detection of initialization loop.

The compiler computes initialization order by finding
a spanning tree between a package's global variables.
But it does so by walking both variables and functions
and stops detecting cycles between variables when they
mix with a cycle of mutually recursive functions.

Fixes issue 4847.

R=golang-dev, daniel.morsing, rsc
CC=golang-dev
https://codereview.appspot.com/9663047
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b