From: Robert Griesemer Date: Wed, 18 Jan 2012 22:10:42 +0000 (-0800) Subject: go/token: replaced Files() with Iterate() X-Git-Tag: weekly.2012-01-20~54 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=9edabbe03832e1203d0819c27542b6316ca39d0d;p=gostls13.git go/token: replaced Files() with Iterate() - Use a faster closure-based iterator rather than a channel-based one. - Otherwise: better code organization, but no other API changes. R=r, r CC=golang-dev https://golang.org/cl/5557051 --- diff --git a/src/pkg/exp/types/check_test.go b/src/pkg/exp/types/check_test.go index ea9218ff51..0e20646a00 100644 --- a/src/pkg/exp/types/check_test.go +++ b/src/pkg/exp/types/check_test.go @@ -47,17 +47,17 @@ var tests = []struct { var fset = token.NewFileSet() -// TODO(gri) This functionality should be in token.Fileset. -func getFile(filename string) *token.File { - for f := range fset.Files() { +func getFile(filename string) (file *token.File) { + fset.Iterate(func(f *token.File) bool { if f.Name() == filename { - return f + file = f + return false // end iteration } - } - return nil + return true + }) + return file } -// TODO(gri) This functionality should be in token.Fileset. func getPos(filename string, offset int) token.Pos { if f := getFile(filename); f != nil { return f.Pos(offset) @@ -65,8 +65,6 @@ func getPos(filename string, offset int) token.Pos { return token.NoPos } -// TODO(gri) Need to revisit parser interface. We should be able to use parser.ParseFiles -// or a similar function instead. func parseFiles(t *testing.T, testname string, filenames []string) (map[string]*ast.File, error) { files := make(map[string]*ast.File) var errors scanner.ErrorList @@ -145,8 +143,6 @@ func eliminate(t *testing.T, expected map[token.Pos]string, errors error) { for _, error := range errors.(scanner.ErrorList) { // error.Pos is a token.Position, but we want // a token.Pos so we can do a map lookup - // TODO(gri) Need to move scanner.Errors over - // to use token.Pos and file set info. pos := getPos(error.Pos.Filename, error.Pos.Offset) if msg, found := expected[pos]; found { // we expect a message at pos; check if it matches diff --git a/src/pkg/go/token/position.go b/src/pkg/go/token/position.go index 8cf3dcd25a..647d1b770b 100644 --- a/src/pkg/go/token/position.go +++ b/src/pkg/go/token/position.go @@ -12,6 +12,9 @@ import ( "sync" ) +// ----------------------------------------------------------------------------- +// Positions + // Position describes an arbitrary source position // including the file, line, and column location. // A Position is valid if the line number is > 0. @@ -81,84 +84,8 @@ func (p Pos) IsValid() bool { return p != NoPos } -func searchFiles(a []*File, x int) int { - return sort.Search(len(a), func(i int) bool { return a[i].base > x }) - 1 -} - -func (s *FileSet) file(p Pos) *File { - // common case: p is in last file touched - if f := s.last; f != nil && f.base <= int(p) && int(p) <= f.base+f.size { - return f - } - // p is not in last file touched - search all files - if i := searchFiles(s.files, int(p)); i >= 0 { - f := s.files[i] - // f.base <= int(p) by definition of searchFiles - if int(p) <= f.base+f.size { - s.last = f - return f - } - } - return nil -} - -// File returns the file which contains the position p. -// If no such file is found (for instance for p == NoPos), -// the result is nil. -// -func (s *FileSet) File(p Pos) (f *File) { - if p != NoPos { - s.mutex.RLock() - f = s.file(p) - s.mutex.RUnlock() - } - return -} - -func (f *File) position(p Pos) (pos Position) { - offset := int(p) - f.base - pos.Offset = offset - pos.Filename, pos.Line, pos.Column = f.info(offset) - return -} - -// 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 -} - -// A lineInfo object describes alternative file and line number -// information (such as provided via a //line comment in a .go -// file) for a given file offset. -type lineInfo struct { - // fields are exported to make them accessible to gob - Offset int - Filename string - Line int -} - -// AddLineInfo adds alternative file and line number information for -// a given file offset. The offset must be larger than the offset for -// the previously added alternative line info and smaller than the -// file size; otherwise the information is ignored. -// -// AddLineInfo is typically used to register alternative position -// information for //line filename:line comments in source files. -// -func (f *File) AddLineInfo(offset int, filename string, line int) { - f.set.mutex.Lock() - if i := len(f.infos); i == 0 || f.infos[i-1].Offset < offset && offset < f.size { - f.infos = append(f.infos, lineInfo{offset, filename, line}) - } - f.set.mutex.Unlock() -} +// ----------------------------------------------------------------------------- +// File // A File is a handle for a file belonging to a FileSet. // A File has a name, size, and line offset table. @@ -253,6 +180,32 @@ func (f *File) SetLinesForContent(content []byte) { f.set.mutex.Unlock() } +// A lineInfo object describes alternative file and line number +// information (such as provided via a //line comment in a .go +// file) for a given file offset. +type lineInfo struct { + // fields are exported to make them accessible to gob + Offset int + Filename string + Line int +} + +// AddLineInfo adds alternative file and line number information for +// a given file offset. The offset must be larger than the offset for +// the previously added alternative line info and smaller than the +// file size; otherwise the information is ignored. +// +// AddLineInfo is typically used to register alternative position +// information for //line filename:line comments in source files. +// +func (f *File) AddLineInfo(offset int, filename string, line int) { + f.set.mutex.Lock() + if i := len(f.infos); i == 0 || f.infos[i-1].Offset < offset && offset < f.size { + f.infos = append(f.infos, lineInfo{offset, filename, line}) + } + f.set.mutex.Unlock() +} + // Pos returns the Pos value for the given file offset; // the offset must be <= f.Size(). // f.Pos(f.Offset(p)) == p. @@ -283,41 +236,6 @@ func (f *File) Line(p Pos) int { return f.Position(p).Line } -// Position returns the Position value for the given file position p; -// p must be a Pos value in that file or NoPos. -// -func (f *File) Position(p Pos) (pos Position) { - if p != NoPos { - if int(p) < f.base || int(p) > f.base+f.size { - panic("illegal Pos value") - } - pos = f.position(p) - } - return -} - -func searchInts(a []int, x int) int { - // This function body is a manually inlined version of: - // - // return sort.Search(len(a), func(i int) bool { return a[i] > x }) - 1 - // - // With better compiler optimizations, this may not be needed in the - // future, but at the moment this change improves the go/printer - // benchmark performance by ~30%. This has a direct impact on the - // speed of gofmt and thus seems worthwhile (2011-04-29). - i, j := 0, len(a) - for i < j { - h := i + (j-i)/2 // avoid overflow when computing h - // i ≤ h < j - if a[h] <= x { - i = h + 1 - } else { - j = h - } - } - return i - 1 -} - func searchLineInfos(a []lineInfo, x int) int { return sort.Search(len(a), func(i int) bool { return a[i].Offset > x }) - 1 } @@ -341,6 +259,29 @@ func (f *File) info(offset int) (filename string, line, column int) { return } +func (f *File) position(p Pos) (pos Position) { + offset := int(p) - f.base + pos.Offset = offset + pos.Filename, pos.Line, pos.Column = f.info(offset) + return +} + +// Position returns the Position value for the given file position p; +// p must be a Pos value in that file or NoPos. +// +func (f *File) Position(p Pos) (pos Position) { + if p != NoPos { + if int(p) < f.base || int(p) > f.base+f.size { + panic("illegal Pos value") + } + pos = f.position(p) + } + return +} + +// ----------------------------------------------------------------------------- +// FileSet + // A FileSet represents a set of source files. // Methods of file sets are synchronized; multiple goroutines // may invoke them concurrently. @@ -404,23 +345,91 @@ func (s *FileSet) AddFile(filename string, base, size int) *File { return f } -// Files returns the files added to the file set. -func (s *FileSet) Files() <-chan *File { - ch := make(chan *File) - go func() { - for i := 0; ; i++ { - var f *File - s.mutex.RLock() - if i < len(s.files) { - f = s.files[i] - } - s.mutex.RUnlock() - if f == nil { - break - } - ch <- f +// Iterate calls f for the files in the file set in the order they were added +// until f returns false. +// +func (s *FileSet) Iterate(f func(*File) bool) { + for i := 0; ; i++ { + var file *File + s.mutex.RLock() + if i < len(s.files) { + file = s.files[i] + } + s.mutex.RUnlock() + if file == nil || !f(file) { + break } - close(ch) - }() - return ch + } +} + +func searchFiles(a []*File, x int) int { + return sort.Search(len(a), func(i int) bool { return a[i].base > x }) - 1 +} + +func (s *FileSet) file(p Pos) *File { + // common case: p is in last file + if f := s.last; f != nil && f.base <= int(p) && int(p) <= f.base+f.size { + return f + } + // p is not in last file - search all files + if i := searchFiles(s.files, int(p)); i >= 0 { + f := s.files[i] + // f.base <= int(p) by definition of searchFiles + if int(p) <= f.base+f.size { + s.last = f + return f + } + } + return nil +} + +// File returns the file that contains the position p. +// If no such file is found (for instance for p == NoPos), +// the result is nil. +// +func (s *FileSet) File(p Pos) (f *File) { + if p != NoPos { + s.mutex.RLock() + f = s.file(p) + s.mutex.RUnlock() + } + return +} + +// 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 +} + +// ----------------------------------------------------------------------------- +// Helper functions + +func searchInts(a []int, x int) int { + // This function body is a manually inlined version of: + // + // return sort.Search(len(a), func(i int) bool { return a[i] > x }) - 1 + // + // With better compiler optimizations, this may not be needed in the + // future, but at the moment this change improves the go/printer + // benchmark performance by ~30%. This has a direct impact on the + // speed of gofmt and thus seems worthwhile (2011-04-29). + // TODO(gri): Remove this when compilers have caught up. + i, j := 0, len(a) + for i < j { + h := i + (j-i)/2 // avoid overflow when computing h + // i ≤ h < j + if a[h] <= x { + i = h + 1 + } else { + j = h + } + } + return i - 1 } diff --git a/src/pkg/go/token/position_test.go b/src/pkg/go/token/position_test.go index 30bec59913..160107df40 100644 --- a/src/pkg/go/token/position_test.go +++ b/src/pkg/go/token/position_test.go @@ -167,12 +167,13 @@ func TestFiles(t *testing.T) { for i, test := range tests { fset.AddFile(test.filename, fset.Base(), test.size) j := 0 - for g := range fset.Files() { - if g.Name() != tests[j].filename { - t.Errorf("expected filename = %s; got %s", tests[j].filename, g.Name()) + fset.Iterate(func(f *File) bool { + if f.Name() != tests[j].filename { + t.Errorf("expected filename = %s; got %s", tests[j].filename, f.Name()) } j++ - } + return true + }) if j != i+1 { t.Errorf("expected %d files; got %d", i+1, j) }