gmane.comp.lang.go.devel

Re: code review 9680043: fmt.Printf: introduce notation for random access to arg... (issue 9680043)

roger peppe — 2013-05-23T13:27:32

why one-indexed?

On 22 May 2013 23:45,  <r< at >golang.org> wrote:

Re: code review 9355044: go.net/iana: new package (issue 9355044)

2013-05-23T11:02:47

ping

https://codereview.appspot.com/9355044/

Re: runtime integrated network poller for the rest of us, windows and BSD variants

Mikio Hara — 2013-05-23T10:50:52


It's C stuff, so using typedef looks fine to me.

Re: runtime integrated network poller for the rest of us, windows and BSD variants

Mikio Hara — 2013-05-23T10:45:43


Thanks.


Ack.

Re: Unexpected failure in tests on plan9/386

Lucio De Re — 2013-05-23T09:41:48

My gut feel is that there are timing issues that impact Plan 9 like
this, but I'm usually wrong on that score.

Christopher, I'd like you to post a message along the lines of this
one to 9fans, my impression is that the subject of this one doesn't do
it justice on golang-dev.

I have encountered all the problems you mentioned, but I have not had
a single successful run so far.  Even my fastest computer fails to
complete a test run.  Still I think that some things do not complete
within expected time limits.

Lucio.

PS: Please do request passing over path/filepath for Plan 9, no point
painting ourselves into a corner.


On 5/23/13, Christopher Nielsen <m4dh4tt3r-Re5JQEeQqe8AvxtiuMwx3w< at >public.gmane.org> wrote:

Re: Re: code review 9648043: container/alloc: add Cache component (issue 9648043)

David Symonds — 2013-05-23T08:38:24


Any form of generics in Go would almost definitely need to wait for
"Go 2", at which point backward compatibility would not constrain us.

Re: code review 9668043: log/syslog: report errors from write (issue 9668043)

2013-05-23T07:56:08

For some reason this change or its friend has caused the tests to take
very long when run with the race detector:

https://code.google.com/p/go/issues/detail?id=5547

https://codereview.appspot.com/9668043/

Re: code review 9662046: cmd/gc: use escape analysis result for make([]T, constant (issue 9662046)

Dave Cheney — 2013-05-23T07:53:04

Hmm, I thought that 1ULL<<16 would be common constant, but there are
no other occurrences, and plenty of other uncommented 1ULL<< shifts.
Please ignore my request.

On Thu, May 23, 2013 at 4:11 PM,  <remyoudompheng-Re5JQEeQqe8AvxtiuMwx3w< at >public.gmane.org> wrote:

Re: code review 9648043: container/alloc: add Cache component (issue 9648043)

Dmitry Vyukov — 2013-05-23T07:47:19

It's always possible to wrap it in a particular case, if it's a problem:

type MyCache alloc.Cache
func (c *MyCache) Put(v *Foo) {
  (*alloc.Cache)(c).Put(v)
}
func (c *MyCache) Take() *Foo {
  return (*alloc.Cache)(c).Take().(*Foo)
}


On Thu, May 23, 2013 at 11:42 AM, Brad Fitzpatrick <bradfitz-iFWiy5xATs8dnm+yROfE0A< at >public.gmane.org> wrote:

Re: code review 9648043: container/alloc: add Cache component (issue 9648043)

Dave Cheney — 2013-05-23T07:45:29

Yes, I am proposing not doing anything. I know this is not a popular
or useful position.

On Thu, May 23, 2013 at 5:42 PM, Brad Fitzpatrick <bradfitz-iFWiy5xATs8dnm+yROfE0A< at >public.gmane.org> wrote:

Re: code review 9648043: container/alloc: add Cache component (issue 9648043)

Brad Fitzpatrick — 2013-05-23T07:42:07

Huh?

What do you propose as an alternative?

Go doesn't have generics.



On Thu, May 23, 2013 at 12:38 AM, <dave-7L4Cwp9BzA+sTnJN9+BGXg< at >public.gmane.org> wrote:

Re: code review 9648043: container/alloc: add Cache component (issue 9648043)

2013-05-23T07:38:14

Dmitry, thank you for working on this. Without trying to pour cold water
on your efforts I would not like to see any API that encouraged the use
of interface{} for Put and Take. I know that with the impasse over
generics there are few other options, but I would prefer to not add an
API that uses interface{} as that would prevent us from defining a
better API with a generic type later.

https://codereview.appspot.com/9648043/

Re: code review 9648043: container/alloc: add Cache component (issue 9648043)

2013-05-23T07:31:59

https://codereview.appspot.com/9648043/diff/9001/src/pkg/container/alloc/cache.go


https://codereview.appspot.com/9648043/diff/9001/src/pkg/container/alloc/cache.go#newcode16



blank line


https://codereview.appspot.com/9648043/diff/9001/src/pkg/container/alloc/cache.go#newcode39
return and

Done. PTAL.
I am happy to replace all comments with your versions :)

https://codereview.appspot.com/9648043/

Re: code review 9373046: syscall: add IPv6 socket options for Unix variants (issue 9373046)

2013-05-23T07:22:16

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

syscall: add IPv6 socket options for Unix variants

This CL adds missing IPv6 socket options which are required
to control IPv6 as described in RFC 3493, RFC 3542.

Update issue 5538

R=golang-dev, dave, iant
CC=golang-dev
https://codereview.appspot.com/9373046


https://codereview.appspot.com/9373046/

Re: code review 9574043: cgo: do not output empty struct (issue 9574043)

2013-05-23T07:07:24

Done https://codereview.appspot.com/9679046/

Alex

https://codereview.appspot.com/9574043/

Re: code review 9574043: cgo: do not output empty struct (issue 9574043)

2013-05-23T07:06:31

Hello golang-dev-/JYPxA39Uh5TLH3MbocFFw< at >public.gmane.org, iant-iFWiy5xATs8dnm+yROfE0A< at >public.gmane.org (cc:
golang-dev-/JYPxA39Uh5TLH3MbocFFw< at >public.gmane.org),

Please take another look.


https://codereview.appspot.com/9574043/

code review 9679046: runtime: update defs_windows_*.h files (issue 9679046)

2013-05-23T07:06:09

Reviewers: golang-dev1,

Message:
Hello golang-dev-/JYPxA39Uh5TLH3MbocFFw< at >public.gmane.org,

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


Description:
runtime: update defs_windows_*.h files

New version of cmd/cgo from
https://codereview.appspot.com/9574043/
is used to regenerate these.

Please review this at https://codereview.appspot.com/9679046/

Affected files:
   M src/pkg/runtime/defs_windows_386.h
   M src/pkg/runtime/defs_windows_amd64.h

Re: code review 9223046: cmd/5l, cmd/6l, cmd/8l, cmd/gc, runtime: generate and u... (issue 9223046)

2013-05-23T06:25:09

https://codereview.appspot.com/9223046/diff/62001/src/cmd/gc/pgen.c
File src/cmd/gc/pgen.c (right):

https://codereview.appspot.com/9223046/diff/62001/src/cmd/gc/pgen.c#newcode309
src/cmd/gc/pgen.c:309: if(thistype != nil)
On 2013/05/22 23:19:11, cshapiro1 wrote:
complete
this


The this parameter shows up in dcl as a node with class PPARAM. The
initial offset of the parameters should be in their xoffset field.

You're doing a lot of extra work trying to figure out how the arguments
are arranged, when we already have this information in the AST.

https://codereview.appspot.com/9223046/

Re: Unexpected failure in tests on plan9/386

Christopher Nielsen — 2013-05-23T06:20:47

I don't know it's significant. I can post to 9fans about it. Or you
can. Doesn't matter to me.

There's something fishy going on. I got all the tests to pass on real
hardware with sse2 except path/filepath because plan9 doesn't support
symlinks (I can submit a CL to special-case plan9 for that test).
Under VMware, I had to compile with GO386=387 and saw what I reported
earlier. Running the tests again on bare metal has been giving me
inconsistent results. Sometimes everything but path/filepath passes
and sometimes random packages fail. The panic message I get for
packages that fail is

suicide: sys: trap: fault write addr=0xfffffffc pc=0x0001e93a
panic: runtime error: index out of range

The pc is different depending on the test, but the rest is the same.

My latest run came out clean.

On Wed, May 22, 2013 at 9:13 PM, Lucio De Re <lucio.dere-Re5JQEeQqe8AvxtiuMwx3w< at >public.gmane.org> wrote:

Re: code review 9662046: cmd/gc: use escape analysis result for make([]T, constant (issue 9662046)

2013-05-23T06:11:20

(fixed typo in description)


https://codereview.appspot.com/9662046/diff/4001/src/cmd/gc/walk.c
File src/cmd/gc/walk.c (right):

https://codereview.appspot.com/9662046/diff/4001/src/cmd/gc/walk.c#newcode1256
src/cmd/gc/walk.c:1256: && mpgetfix(r->val.u.xval) < (1ULL<<16) /
t->type->width) {
On 2013/05/23 04:48:52, dfc wrote:

Maybe, but it would need a name. The constant is used in the ONEW case,
it's the size beyond which we ignore escape analysis and allocate on
heap anyway.

https://codereview.appspot.com/9662046/

Re: code review 9223046: cmd/5l, cmd/6l, cmd/8l, cmd/gc, runtime: generate and u... (issue 9223046)

2013-05-23T05:28:58

https://codereview.appspot.com/9223046/diff/62001/src/pkg/runtime/mgc0.c
File src/pkg/runtime/mgc0.c (right):

https://codereview.appspot.com/9223046/diff/62001/src/pkg/runtime/mgc0.c#newcode1397
src/pkg/runtime/mgc0.c:1397: // locals need to be scanned.
On 2013/05/22 23:19:11, cshapiro1 wrote:
(virtual)
arguments

You "Scan the entire stack frame" and then you additionally scan args in
"if(f->args > 0) {".
What is true?
1. The comment is incorrect.
2. The code under "if" is not supposed to be executed in this case.
3. Args is not a part of the entire stack frame.
4. I am missing something.

https://codereview.appspot.com/9223046/diff/62001/src/pkg/runtime/mgc0.c#newcode1425
src/pkg/runtime/mgc0.c:1425: for((j = (rem < 32) ? rem : 32); j > 0;
j--) {
On 2013/05/22 23:19:11, cshapiro1 wrote:
common bit
performance

There is no other bitmap scanning in GC AFAIR.

https://codereview.appspot.com/9223046/diff/75001/src/pkg/runtime/symtab.c
File src/pkg/runtime/symtab.c (right):

https://codereview.appspot.com/922