code review 9269043: go/token: let FileSet.AddFile take a negative base

go/token: let FileSet.AddFile take a negative base

Negative base now means "automatic". Fixes a higher
level race.

Fixes issue 5418

[gri, 2013-05-07 17:36:52]

LGTM

https://codereview.appspot.com/9269043/diff/2001/src/pkg/go/parser/parser.go
File src/pkg/go/parser/parser.go (right):

https://codereview.appspot.com/9269043/diff/2001/src/pkg/go/parser/parser.go#...
src/pkg/go/parser/parser.go:67: p.file = fset.AddFile(filename, -1, len(src))
There's probably a bunch of places where we do this.

[bradfitz, 2013-05-07 18:46:06]

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

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

[gri, 2013-05-07 19:37:30]

LGTM

but please fix the size check

https://codereview.appspot.com/9269043/diff/8001/src/pkg/go/token/position.go
File src/pkg/go/token/position.go (right):

https://codereview.appspot.com/9269043/diff/8001/src/pkg/go/token/position.go...
src/pkg/go/token/position.go:317: // size must not be negative. As a special
case, if base is negative, the
Maybe:

As a special case, if a negative base is provided, the current value of the
FileSet's Base() is used instead.

https://codereview.appspot.com/9269043/diff/8001/src/pkg/go/token/position.go...
src/pkg/go/token/position.go:333: if base < 0 {
This misses the size check if base < 0. Maybe:

if base < 0 {
  base = s.base
}
if base < s.base || size < 0 ...

[bradfitz, 2013-05-07 20:25:51]

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

Please take another look.

[gri, 2013-05-07 20:31:54]

LGTM

[bradfitz, 2013-05-14 16:30:17]

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

go/token: let FileSet.AddFile take a negative base

Negative base now means "automatic". Fixes a higher
level race.

Fixes issue 5418

R=gri
CC=golang-dev
https://codereview.appspot.com/9269043

[dvyukov, 2013-05-14 16:42:40]

Are we already unfreezed?

[bradfitz, 2013-05-14 16:45:33]

I was told "submit away"

Go 1.1 is branched.




On Tue, May 14, 2013 at 9:42 AM, Dmitry Vyukov <dvyukov@google.com> wrote:

> Are we already unfreezed?
>

[r, 2013-05-14 16:50:55]

I'd prefer to take it easy but with the mad rush I feel like a
one-armed traffic cop controlling visitor's day at the nudist colony.

-rob