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

Issue 12030044: code review 12030044: go.tools/ssa: fix a package-level var initialization ... (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 8 months ago by adonovan
Modified:
10 years, 8 months ago
Reviewers:
gri
CC:
gri, golang-dev
Visibility:
Public.

Description

go.tools/ssa: fix a package-level var initialization order bug. buildDecl was visiting all decls in source order, but the spec calls for visiting all vars and init() funcs in order, then all remaining functions. These two passes are now called buildInit(), buildFuncDecl(). + Test. Also: - Added workaround to gcimporter for Func with pkg==nil. - Prog.concreteMethods has been merged into Pkg.values. - Prog.concreteMethod() renamed declaredFunc(). - s/mfunc/obj/ (name cleanup from recent gri CL)

Patch Set 1 #

Patch Set 2 : diff -r eb1be81dc991 https://code.google.com/p/go.tools #

Patch Set 3 : diff -r eb1be81dc991 https://code.google.com/p/go.tools #

Patch Set 4 : diff -r eb1be81dc991 https://code.google.com/p/go.tools #

Patch Set 5 : diff -r eb1be81dc991 https://code.google.com/p/go.tools #

Total comments: 1

Patch Set 6 : diff -r eb1be81dc991 https://code.google.com/p/go.tools #

Unified diffs Side-by-side diffs Delta from patch set Stats (+139 lines, -97 lines) Patch
M go/types/gcimporter.go View 1 2 3 2 chunks +8 lines, -0 lines 0 comments Download
M ssa/builder.go View 1 2 3 5 chunks +45 lines, -53 lines 0 comments Download
M ssa/create.go View 1 2 3 4 2 chunks +4 lines, -11 lines 0 comments Download
M ssa/interp/interp_test.go View 1 1 chunk +1 line, -0 lines 0 comments Download
M ssa/interp/testdata/ifaceprom.go View 1 1 chunk +1 line, -1 line 0 comments Download
A ssa/interp/testdata/initorder.go View 1 1 chunk +55 lines, -0 lines 0 comments Download
M ssa/promote.go View 1 5 chunks +18 lines, -20 lines 0 comments Download
M ssa/source.go View 1 1 chunk +1 line, -5 lines 0 comments Download
M ssa/ssa.go View 1 2 chunks +6 lines, -7 lines 0 comments Download

Messages

Total messages: 3
adonovan
Hello gri@golang.org (cc: golang-dev@googlegroups.com), I'd like you to review this change to https://code.google.com/p/go.tools
10 years, 8 months ago (2013-07-29 18:13:55 UTC) #1
gri
LGTM https://codereview.appspot.com/12030044/diff/10001/ssa/promote.go File ssa/promote.go (right): https://codereview.appspot.com/12030044/diff/10001/ssa/promote.go#newcode122 ssa/promote.go:122: // Panic ensues if there is none. very ...
10 years, 8 months ago (2013-07-29 18:20:06 UTC) #2
adonovan
10 years, 8 months ago (2013-07-29 18:24:11 UTC) #3
*** Submitted as
https://code.google.com/p/go/source/detail?r=b24277f5d911&repo=tools ***

go.tools/ssa: fix a package-level var initialization order bug.

buildDecl was visiting all decls in source order, but the spec
calls for visiting all vars and init() funcs in order, then
all remaining functions.  These two passes are now called
buildInit(), buildFuncDecl().

+ Test.

Also:
- Added workaround to gcimporter for Func with pkg==nil.
- Prog.concreteMethods has been merged into Pkg.values.
- Prog.concreteMethod() renamed declaredFunc().
- s/mfunc/obj/ (name cleanup from recent gri CL)

R=gri
CC=golang-dev
https://codereview.appspot.com/12030044
Sign in to reply to this message.

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