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

Issue 53820043: code review 53820043: cmd/link: pclntab generation (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years, 1 month ago by rsc
Modified:
11 years, 1 month ago
Reviewers:
iant
CC:
iant, golang-codereviews
Visibility:
Public.

Description

cmd/link: pclntab generation

Patch Set 1 #

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

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

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

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

Total comments: 16

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

Total comments: 2

Patch Set 7 : diff -r 30b337011ce5 https://code.google.com/p/go/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+2744 lines, -16 lines) Patch
A src/cmd/link/pclntab.go View 1 2 3 4 5 1 chunk +480 lines, -0 lines 0 comments Download
A src/cmd/link/pclntab_test.go View 1 2 3 4 5 1 chunk +334 lines, -0 lines 0 comments Download
M src/cmd/link/prog.go View 1 2 chunks +2 lines, -0 lines 0 comments Download
M src/cmd/link/runtime.go View 1 1 chunk +17 lines, -0 lines 0 comments Download
A src/cmd/link/testdata/Makefile View 1 1 chunk +10 lines, -0 lines 0 comments Download
A src/cmd/link/testdata/genpcln.go View 1 2 3 1 chunk +112 lines, -0 lines 0 comments Download
M src/cmd/link/testdata/link.hello.darwin.amd64 View 1 1 chunk +38 lines, -16 lines 0 comments Download
A src/cmd/link/testdata/pclntab.s View 1 1 chunk +1751 lines, -0 lines 0 comments Download

Messages

Total messages: 5
rsc
Hello iant (cc: golang-codereviews@googlegroups.com), I'd like you to review this change to https://code.google.com/p/go/
11 years, 1 month ago (2014-01-17 19:17:43 UTC) #1
iant
https://codereview.appspot.com/53820043/diff/70001/src/cmd/link/pclntab.go File src/cmd/link/pclntab.go (right): https://codereview.appspot.com/53820043/diff/70001/src/cmd/link/pclntab.go#newcode6 src/cmd/link/pclntab.go:6: // See runtime.go. Why should I see runtime.go? https://codereview.appspot.com/53820043/diff/70001/src/cmd/link/pclntab.go#newcode104 ...
11 years, 1 month ago (2014-01-17 21:16:19 UTC) #2
rsc
PTAL https://codereview.appspot.com/53820043/diff/70001/src/cmd/link/pclntab.go File src/cmd/link/pclntab.go (right): https://codereview.appspot.com/53820043/diff/70001/src/cmd/link/pclntab.go#newcode6 src/cmd/link/pclntab.go:6: // See runtime.go. On 2014/01/17 21:16:19, iant wrote: ...
11 years, 1 month ago (2014-01-21 16:42:18 UTC) #3
iant
LGTM https://codereview.appspot.com/53820043/diff/90001/src/cmd/link/pclntab.go File src/cmd/link/pclntab.go (right): https://codereview.appspot.com/53820043/diff/90001/src/cmd/link/pclntab.go#newcode168 src/cmd/link/pclntab.go:168: // The first entry gives 1+n, the size ...
11 years, 1 month ago (2014-01-21 17:24:37 UTC) #4
rsc
11 years, 1 month ago (2014-01-21 18:47:51 UTC) #5
*** Submitted as https://code.google.com/p/go/source/detail?r=95018395386a ***

cmd/link: pclntab generation

R=iant
CC=golang-codereviews
https://codereview.appspot.com/53820043

https://codereview.appspot.com/53820043/diff/90001/src/cmd/link/pclntab.go
File src/cmd/link/pclntab.go (right):

https://codereview.appspot.com/53820043/diff/90001/src/cmd/link/pclntab.go#ne...
src/cmd/link/pclntab.go:168: // The first entry gives 1+n, the size of the
array.
On 2014/01/21 17:24:37, iant wrote:
> I don't see how to reconcile this with http://golang.org/s/go12symtab, which
> says this table is
> N name1 name2 name3 … nameN
> If this is correct then I guess go12symtab should say
> N+1 name1 name2 name3 … nameN

Done.
Sign in to reply to this message.

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