code review 5015051: go/build: change //build to // +build

go/build: change //build to // +build

New rules as discussed on CL 5011046.
Also apply to C and assembly files, not just Go files.

[rsc, 2011-09-15 18:56:43]

Hello r (cc: golang-dev@googlegroups.com),

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

[r, 2011-09-15 19:27:36]

LGTM

http://codereview.appspot.com/5015051/diff/2001/src/pkg/go/build/dir.go
File src/pkg/go/build/dir.go (right):

http://codereview.appspot.com/5015051/diff/2001/src/pkg/go/build/dir.go#newco...
src/pkg/go/build/dir.go:339: f :=
strings.Fields(string(bytes.TrimSpace(line[len(slashslash):])))
why not use bytes.Fields?

[rog, 2011-09-15 19:39:52]

http://codereview.appspot.com/5015051/diff/2001/src/pkg/go/build/dir.go
File src/pkg/go/build/dir.go (right):

http://codereview.appspot.com/5015051/diff/2001/src/pkg/go/build/dir.go#newco...
src/pkg/go/build/dir.go:339: f :=
strings.Fields(string(bytes.TrimSpace(line[len(slashslash):])))
On 2011/09/15 19:27:37, r wrote:
> why not use bytes.Fields?

one allocation vs len(f) allocations?

[r, 2011-09-15 19:41:44]

On 2011/09/15 19:39:52, rog wrote:
> http://codereview.appspot.com/5015051/diff/2001/src/pkg/go/build/dir.go
> File src/pkg/go/build/dir.go (right):
> 
>
http://codereview.appspot.com/5015051/diff/2001/src/pkg/go/build/dir.go#newco...
> src/pkg/go/build/dir.go:339: f :=
> strings.Fields(string(bytes.TrimSpace(line[len(slashslash):])))
> On 2011/09/15 19:27:37, r wrote:
> > why not use bytes.Fields?
> 
> one allocation vs len(f) allocations?

i believe they both do one allocation.

[rog, 2011-09-15 19:44:17]

On 2011/09/15 19:41:44, r wrote:
> On 2011/09/15 19:39:52, rog wrote:
> > http://codereview.appspot.com/5015051/diff/2001/src/pkg/go/build/dir.go
> > File src/pkg/go/build/dir.go (right):
> > 
> >
>
http://codereview.appspot.com/5015051/diff/2001/src/pkg/go/build/dir.go#newco...
> > src/pkg/go/build/dir.go:339: f :=
> > strings.Fields(string(bytes.TrimSpace(line[len(slashslash):])))
> > On 2011/09/15 19:27:37, r wrote:
> > > why not use bytes.Fields?
> > 
> > one allocation vs len(f) allocations?
> 
> i believe they both do one allocation.

but you'd have to convert the byte slice for each field to string in order to
pass it to matchOSArch. unless i'm missing something.

[r, 2011-09-15 19:46:34]

i suppose

anyway i did LGTM it

[rsc, 2011-09-15 20:32:05]

On Thu, Sep 15, 2011 at 15:27,  <r@golang.org> wrote:
> why not use bytes.Fields?

eventually i needed strings to call some other
functions (like ctxt.matchOSArch).  i just delayed
the conversion until i was sure i would need it.
i delayed it a little more and now it looks almost normal.

		if bytes.HasPrefix(line, slashslash) {
			line = bytes.TrimSpace(line[len(slashslash):])
			if len(line) > 0 && line[0] == '+' {
				// Looks like a comment +line.
				f := strings.Fields(string(line))

[rsc, 2011-09-15 20:48:26]

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

go/build: change //build to // +build

New rules as discussed on CL 5011046.
Also apply to C and assembly files, not just Go files.

R=r, rogpeppe
CC=golang-dev
http://codereview.appspot.com/5015051

[rog, 2011-09-15 20:56:47]

definitely nicer. do i sense an idiom emerging? the dual between
bytes and strings is quite nice for this kind of transformation.

On 15 September 2011 21:32, Russ Cox <rsc@golang.org> wrote:
> On Thu, Sep 15, 2011 at 15:27,  <r@golang.org> wrote:
>> why not use bytes.Fields?
>
> eventually i needed strings to call some other
> functions (like ctxt.matchOSArch).  i just delayed
> the conversion until i was sure i would need it.
> i delayed it a little more and now it looks almost normal.
>
>                if bytes.HasPrefix(line, slashslash) {
>                        line = bytes.TrimSpace(line[len(slashslash):])
>                        if len(line) > 0 && line[0] == '+' {
>                                // Looks like a comment +line.
>                                f := strings.Fields(string(line))
>