r | 29 Jun 02:59 2011

code review 4639083: reflect: MethodByName (issue4639083)

Reviewers: rsc,

Message:
Hello rsc (cc: golang-dev@...),

I'd like you to review this change to
https://go.googlecode.com/hg/

Description:
reflect: MethodByName
It's more common to ask for methods by name than by index, so might
as well make it easy to do so.

Please review this at http://codereview.appspot.com/4639083/

Affected files:
   M src/pkg/reflect/all_test.go
   M src/pkg/reflect/type.go
   M src/pkg/reflect/value.go

rsc | 29 Jun 03:47 2011

Re: code review 4639083: reflect: MethodByName (issue4639083)

some simplifications

http://codereview.appspot.com/4639083/diff/2001/src/pkg/reflect/type.go
File src/pkg/reflect/type.go (right):

http://codereview.appspot.com/4639083/diff/2001/src/pkg/reflect/type.go#newcode356
src/pkg/reflect/type.go:356: Func    Value
Please add

Index int

like StructField has, so that if a caller goes
to the effort of using Type.MethodByName(name)
it has the index ready to use with Value.Method(i).

http://codereview.appspot.com/4639083/diff/2001/src/pkg/reflect/type.go#newcode463
src/pkg/reflect/type.go:463: m.Func = valueFromIword(flag, m.Type,
iword(fn))
a
m.Index = i
.

http://codereview.appspot.com/4639083/diff/2001/src/pkg/reflect/type.go#newcode479
src/pkg/reflect/type.go:479: ok = true
return t.Method(i), true

http://codereview.appspot.com/4639083/diff/2001/src/pkg/reflect/type.go#newcode483
src/pkg/reflect/type.go:483: if !ok {
return

(Continue reading)

brainman | 29 Jun 03:59 2011
Picon

Re: How to setup a windows-amd64 build?

I didn't think about it properly, but you could, probably, use 32 bit mingw and cross-compile (if you ignore any cgo related bits). I will, certainly, be using linux-386 to build and just test on 64 bit Windows for now.

Alex

brainman | 29 Jun 04:04 2011
Picon

Re: windows-386-ec2 broken by gotest: add -test.benchtime and -test.cpu flags.

On Wednesday, 29 June 2011 02:43:49 UTC+10, Brad Fitzpatrick wrote:


I think the builder needs a -maxbuildtime=<minutes> flag, at least for Windows.


We could, certainly, try, but, I suspect, we'll discover some nasty things - you kill "bash", but "make" processes that were started by it will still run, or something of that kind.

Alex
mikioh.mikioh | 29 Jun 04:21 2011
Picon

code review 4629082: runtime/cgo: fix build (issue4629082)

Reviewers: golang-dev_googlegroups.com,

Message:
Hello golang-dev@... (cc: golang-dev@...),

I'd like you to review this change to
https://go.googlecode.com/hg/

Description:
runtime/cgo: fix build

Please review this at http://codereview.appspot.com/4629082/

Affected files:
   M src/pkg/runtime/cgo/darwin_386.c

Index: src/pkg/runtime/cgo/darwin_386.c
===================================================================
--- a/src/pkg/runtime/cgo/darwin_386.c
+++ b/src/pkg/runtime/cgo/darwin_386.c
 <at>  <at>  -120,7 +120,7  <at>  <at> 
  	ts->g->stackguard = size;
  	err = pthread_create(&p, &attr, threadentry, ts);
  	if (err != 0) {
-		fprintf(stderr, "runtime/cgo: pthread_create failed: %s\n",  
strerror(error));
+		fprintf(stderr, "runtime/cgo: pthread_create failed: %s\n",  
strerror(err));
  		abort();
  	}
  }

zhai | 29 Jun 04:21 2011
Picon

Re: Re: code review 4639077: ld: fix ELF strip by removing overlap of sections (issue4639077)

There are still some overlapped sections in binary file "goinstall" and "godoc"


GOARCH=386
godoc:
.rodata 8177000-825c96b  overlaps .dynsym 81772cc-81773eb
.rodata 8177000-825c96b  overlaps .dynstr 81773ec-8177517
.rodata 8177000-825c96b  overlaps .gnu.version 8177008-817702b
.rodata 8177000-825c96b  overlaps .gnu.version_r 817702c-817708b
.rodata 8177000-825c96b  overlaps .rel.plt 81770f4-817715b
.rodata 8177000-825c96b  overlaps .plt 81771ec-81772cb
.rodata 8177000-825c96b  overlaps .hash 817708c-81770f3
.rodata 8177000-825c96b  overlaps .rel 8177000-8177007
.data 8340000-83428d7  overlaps .got.plt 83401c0-83401ff

goinstall:
.rodata 813d000-81fe3af  overlaps .dynsym 813d2cc-813d3eb
.rodata 813d000-81fe3af  overlaps .dynstr 813d3ec-813d517
.rodata 813d000-81fe3af  overlaps .gnu.version 813d008-813d02b
.rodata 813d000-81fe3af  overlaps .gnu.version_r 813d02c-813d08b
.rodata 813d000-81fe3af  overlaps .rel.plt 813d0f4-813d15b
.rodata 813d000-81fe3af  overlaps .plt 813d1ec-813d2cb
.rodata 813d000-81fe3af  overlaps .hash 813d08c-813d0f3
.rodata 813d000-81fe3af  overlaps .dynamic 813d15c-813d1eb
.data 82bb000-82bd847  overlaps .got.plt 82bb260-82bb29f


On Wed, Jun 29, 2011 at 5:29 AM, <n13m3y3r-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
Does the spec allow a NOBITS section to be inside a
segment set to LOAD?

Note .bss is also NOBITS.


I also made sure there was no regression on Plan 9. :)

Thanks!

http://codereview.appspot.com/4639077/

Russ Cox | 29 Jun 04:26 2011

Re: code review 4629082: runtime/cgo: fix build (issue4629082)

LGTM

rsc | 29 Jun 04:26 2011

Re: code review 4629082: runtime/cgo: fix build (issue4629082)

*** Submitted as
http://code.google.com/p/go/source/detail?r=a1541ae18b69 ***

runtime/cgo: fix build

R=golang-dev, rsc
CC=golang-dev
http://codereview.appspot.com/4629082

Committer: Russ Cox <rsc@...>

http://codereview.appspot.com/4629082/

r | 29 Jun 04:47 2011

Re: code review 4639083: reflect: MethodByName (issue4639083)

Hello rsc@... (cc: golang-dev@...),

Please take another look.

http://codereview.appspot.com/4639083/

r | 29 Jun 04:52 2011

Re: code review 4639083: reflect: MethodByName (issue4639083)

Hello rsc@... (cc: golang-dev@...),

Please take another look.

http://codereview.appspot.com/4639083/


Gmane