go/token: fix data race on FileSet.last

Fixes #4345.

Benchmarks are promising,

benchmark         old ns/op    new ns/op    delta
BenchmarkPrint     14716391     14747131   +0.21%

benchmark         old ns/op    new ns/op    delta
BenchmarkParse      8846219      8809343   -0.42%

benchmark          old MB/s     new MB/s  speedup
BenchmarkParse         6.61         6.64    1.00x

Also includes additional tests to improve token.FileSet coverage.

R=dvyukov, gri
CC=golang-dev
https://golang.org/cl/6968044

src/pkg/go/token/position.go

@@ -295,9 +295,9 @@ type FileSet struct {
 
 // NewFileSet creates a new file set.
 func NewFileSet() *FileSet {
-	s := new(FileSet)
+	return &FileSet{
-	s.base = 1 // 0 == NoPos
+		base: 1, // 0 == NoPos
-	return s
+	}
 }
 
 // Base returns the minimum base offset that must be provided to
@@ -367,8 +367,10 @@ func searchFiles(a []*File, x int) int {
 }
 
 func (s *FileSet) file(p Pos) *File {
+	s.mutex.RLock()
 	// common case: p is in last file
 	if f := s.last; f != nil && f.base <= int(p) && int(p) <= f.base+f.size {
+		s.mutex.RUnlock()
 		return f
 	}
 	// p is not in last file - search all files
@@ -376,10 +378,14 @@ func (s *FileSet) file(p Pos) *File {
 		f := s.files[i]
 		// f.base <= int(p) by definition of searchFiles
 		if int(p) <= f.base+f.size {
-			s.last = f
+			s.mutex.RUnlock()
+			s.mutex.Lock()
+			s.last = f // race is ok - s.last is only a cache
+			s.mutex.Unlock()
 			return f
 		}
 	}
+	s.mutex.RUnlock()
 	return nil
 }
 
@@ -389,9 +395,7 @@ func (s *FileSet) file(p Pos) *File {
 //
 func (s *FileSet) File(p Pos) (f *File) {
 	if p != NoPos {
-		s.mutex.RLock()
 		f = s.file(p)
-		s.mutex.RUnlock()
 	}
 	return
 }
@@ -399,11 +403,9 @@ func (s *FileSet) File(p Pos) (f *File) {
 // Position converts a Pos in the fileset into a general Position.
 func (s *FileSet) Position(p Pos) (pos Position) {
 	if p != NoPos {
-		s.mutex.RLock()
 		if f := s.file(p); f != nil {
 			pos = f.position(p)
 		}
-		s.mutex.RUnlock()
 	}
 	return
 }

src/pkg/go/token/position_test.go

@@ -182,6 +182,32 @@ func TestFiles(t *testing.T) {
 	}
 }
 
+// FileSet.File should return nil if Pos is past the end of the FileSet.
+func TestFileSetPastEnd(t *testing.T) {
+	fset := NewFileSet()
+	for _, test := range tests {
+		fset.AddFile(test.filename, fset.Base(), test.size)
+	}
+	if f := fset.File(Pos(fset.Base())); f != nil {
+		t.Errorf("expected nil, got %v", f)
+	}
+}
+
+func TestFileSetCacheUnlikely(t *testing.T) {
+	fset := NewFileSet()
+	offsets := make(map[string]int)
+	for _, test := range tests {
+		offsets[test.filename] = fset.Base()
+		fset.AddFile(test.filename, fset.Base(), test.size)
+	}
+	for file, pos := range offsets {
+		f := fset.File(Pos(pos))
+		if f.Name() != file {
+			t.Errorf("expecting %q at position %d, got %q", file, pos, f.Name())
+		}
+	}
+}
+
 // issue 4345. Test concurrent use of FileSet.Pos does not trigger a
 // race in the FileSet position cache.
 func TestFileSetRace(t *testing.T) {