diff --git a/src/pkg/go/token/position.go b/src/pkg/go/token/position.go index fc45c1e769..f5d9995618 100644 --- a/src/pkg/go/token/position.go +++ b/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) - s.base = 1 // 0 == NoPos - return s + return &FileSet{ + base: 1, // 0 == NoPos + } } // 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 } diff --git a/src/pkg/go/token/position_test.go b/src/pkg/go/token/position_test.go index 3e7d552b75..1d36c22268 100644 --- a/src/pkg/go/token/position_test.go +++ b/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) {