FAQ
Reviewers: gri,

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

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


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

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

Fixes issue 5418

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

Affected files:
    M src/pkg/go/parser/parser.go
    M src/pkg/go/token/position.go
    M src/pkg/go/token/position_test.go


Index: src/pkg/go/parser/parser.go
===================================================================
--- a/src/pkg/go/parser/parser.go
+++ b/src/pkg/go/parser/parser.go
@@ -64,7 +64,7 @@
   }

   func (p *parser) init(fset *token.FileSet, filename string, src []byte,
mode Mode) {
- p.file = fset.AddFile(filename, fset.Base(), len(src))
+ p.file = fset.AddFile(filename, -1, len(src))
    var m scanner.Mode
    if mode&ParseComments != 0 {
     m = scanner.ScanComments
Index: src/pkg/go/token/position.go
===================================================================
--- a/src/pkg/go/token/position.go
+++ b/src/pkg/go/token/position.go
@@ -314,7 +314,8 @@
   // AddFile adds a new file with a given filename, base offset, and file
size
   // to the file set s and returns the file. Multiple files may have the same
   // name. The base offset must not be smaller than the FileSet's Base(), and
-// size must not be negative.
+// size must not be negative. As a special case, if base is negative, the
+// base is set to the current value of the FileSet's Base().
   //
   // Adding the file will set the file set's Base() value to base + size + 1
   // as the minimum base value for the next file. The following relationship
@@ -329,7 +330,9 @@
   func (s *FileSet) AddFile(filename string, base, size int) *File {
    s.mutex.Lock()
    defer s.mutex.Unlock()
- if base < s.base || size < 0 {
+ if base < 0 {
+ base = s.base
+ } else if base < s.base || size < 0 {
     panic("illegal base or size")
    }
    // base >= s.base && size >= 0
Index: src/pkg/go/token/position_test.go
===================================================================
--- a/src/pkg/go/token/position_test.go
+++ b/src/pkg/go/token/position_test.go
@@ -167,7 +167,13 @@
   func TestFiles(t *testing.T) {
    fset := NewFileSet()
    for i, test := range tests {
- fset.AddFile(test.filename, fset.Base(), test.size)
+ base := fset.Base()
+ if i%2 == 1 {
+ // Setting a negative base is equivalent to
+ // fset.Base(), so test some of each.
+ base = -1
+ }
+ fset.AddFile(test.filename, base, test.size)
     j := 0
     fset.Iterate(func(f *File) bool {
      if f.Name() != tests[j].filename {


--

---
You received this message because you are subscribed to the Google Groups "golang-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Search Discussions

  • Gri at May 7, 2013 at 7:37 pm
    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#newcode317
    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#newcode333
    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 ...

    https://codereview.appspot.com/9269043/

    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
  • Bradfitz at May 7, 2013 at 8:25 pm
    Hello gri@golang.org (cc: golang-dev@googlegroups.com),

    Please take another look.


    https://codereview.appspot.com/9269043/

    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.
  • Gri at May 7, 2013 at 8:31 pm
    LGTM

    https://codereview.appspot.com/9269043/

    --

    ---
    You received this message because you are subscribed to the Google Groups "golang-dev" group.
    To unsubscribe from this group and stop receiving emails from it, send an email to golang-dev+unsubscribe@googlegroups.com.
    For more options, visit https://groups.google.com/groups/opt_out.

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupgolang-dev @
categoriesgo
postedMay 7, '13 at 6:46p
activeMay 7, '13 at 8:31p
posts4
users2
websitegolang.org

2 users in discussion

Bradfitz: 2 posts Gri: 2 posts

People

Translate

site design / logo © 2022 Grokbase