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

Issue 140880043: code review 140880043: runtime: convert symtab.c into symtab.go (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 6 months ago by rsc
Modified:
10 years, 6 months ago
Reviewers:
gobot, josharian
CC:
golang-codereviews, dave_cheney.net, bradfitz, josharian, iant, khr, r
Visibility:
Public.

Description

runtime: convert symtab.c into symtab.go Because symtab.c was partially converted before, the diffs are not terribly useful. The earlier conversion was trying to refactor or clean up the code in addition to doing the translation. It also made a mistake by redefining Func to be something users could overwrite. I undid those changes, making symtab.go a more literal line-for-line translation of symtab.c instead.

Patch Set 1 #

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

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

Total comments: 4

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

Total comments: 9

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

Unified diffs Side-by-side diffs Delta from patch set Stats (+276 lines, -460 lines) Patch
M src/pkg/runtime/error.go View 1 1 chunk +1 line, -1 line 0 comments Download
M src/pkg/runtime/extern.go View 1 2 3 4 3 chunks +1 line, -12 lines 0 comments Download
M src/pkg/runtime/funcdata.h View 1 1 chunk +2 lines, -0 lines 0 comments Download
M src/pkg/runtime/runtime.c View 1 1 chunk +1 line, -0 lines 0 comments Download
M src/pkg/runtime/string.go View 1 2 3 4 1 chunk +0 lines, -13 lines 0 comments Download
M src/pkg/runtime/stubs.go View 1 2 3 4 1 chunk +7 lines, -11 lines 0 comments Download
R src/pkg/runtime/symtab.c View 1 1 chunk +0 lines, -333 lines 0 comments Download
M src/pkg/runtime/symtab.go View 1 2 3 4 5 chunks +210 lines, -64 lines 0 comments Download
M src/pkg/runtime/traceback.go View 1 2 3 4 7 chunks +54 lines, -26 lines 0 comments Download

Messages

Total messages: 11
rsc
Hello golang-codereviews@googlegroups.com (cc: iant, josharian, khr, r), I'd like you to review this change to ...
10 years, 6 months ago (2014-09-02 23:24:28 UTC) #1
dave_cheney.net
https://codereview.appspot.com/140880043/diff/40001/src/pkg/runtime/symtab.go File src/pkg/runtime/symtab.go (right): https://codereview.appspot.com/140880043/diff/40001/src/pkg/runtime/symtab.go#newcode50 src/pkg/runtime/symtab.go:50: pcln := (*[100]byte)(unsafe.Pointer(&pclntab)) Where does the 100 come from? ...
10 years, 6 months ago (2014-09-02 23:34:27 UTC) #2
rsc
https://codereview.appspot.com/140880043/diff/40001/src/pkg/runtime/symtab.go File src/pkg/runtime/symtab.go (right): https://codereview.appspot.com/140880043/diff/40001/src/pkg/runtime/symtab.go#newcode50 src/pkg/runtime/symtab.go:50: pcln := (*[100]byte)(unsafe.Pointer(&pclntab)) On 2014/09/02 23:34:26, dfc wrote: > ...
10 years, 6 months ago (2014-09-02 23:43:11 UTC) #3
bradfitz
https://codereview.appspot.com/140880043/diff/40001/src/pkg/runtime/traceback.go File src/pkg/runtime/traceback.go (right): https://codereview.appspot.com/140880043/diff/40001/src/pkg/runtime/traceback.go#newcode527 src/pkg/runtime/traceback.go:527: if s[i] == t[0] && hasprefix(s, t) { this ...
10 years, 6 months ago (2014-09-02 23:45:43 UTC) #4
bradfitz
On Tue, Sep 2, 2014 at 4:43 PM, <rsc@golang.org> wrote: > > https://codereview.appspot.com/140880043/diff/40001/src/ > pkg/runtime/symtab.go ...
10 years, 6 months ago (2014-09-02 23:46:25 UTC) #5
rsc
On Tue, Sep 2, 2014 at 7:46 PM, Brad Fitzpatrick <bradfitz@golang.org> wrote: > On Tue, ...
10 years, 6 months ago (2014-09-02 23:49:02 UTC) #6
rsc
PTAL I painted the shed 8. https://codereview.appspot.com/140880043/diff/40001/src/pkg/runtime/traceback.go File src/pkg/runtime/traceback.go (right): https://codereview.appspot.com/140880043/diff/40001/src/pkg/runtime/traceback.go#newcode527 src/pkg/runtime/traceback.go:527: if s[i] == ...
10 years, 6 months ago (2014-09-02 23:50:45 UTC) #7
josharian
LGTM as a line-by-line translation https://codereview.appspot.com/140880043/diff/60001/src/pkg/runtime/runtime.c File src/pkg/runtime/runtime.c (right): https://codereview.appspot.com/140880043/diff/60001/src/pkg/runtime/runtime.c#newcode123 src/pkg/runtime/runtime.c:123: #pragma textflag NOSPLIT Seems ...
10 years, 6 months ago (2014-09-03 16:10:52 UTC) #8
rsc
https://codereview.appspot.com/140880043/diff/60001/src/pkg/runtime/runtime.c File src/pkg/runtime/runtime.c (right): https://codereview.appspot.com/140880043/diff/60001/src/pkg/runtime/runtime.c#newcode123 src/pkg/runtime/runtime.c:123: #pragma textflag NOSPLIT On 2014/09/03 16:10:52, josharian wrote: > ...
10 years, 6 months ago (2014-09-03 16:47:03 UTC) #9
rsc
*** Submitted as https://code.google.com/p/go/source/detail?r=9debaf89fac7 *** runtime: convert symtab.c into symtab.go Because symtab.c was partially converted ...
10 years, 6 months ago (2014-09-03 17:02:52 UTC) #10
gobot
10 years, 6 months ago (2014-09-03 17:27:30 UTC) #11
Message was sent while issue was closed.
This CL appears to have broken the windows-amd64-perf builder.
See http://build.golang.org/log/5e7d5a7fe7af7db286e404711be33c008094a5bc
Sign in to reply to this message.

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