Commit 28cc8aa8 authored by Robert Griesemer's avatar Robert Griesemer

go/printer: measure lines/construct in generated output rather than incoming source

No change to $GOROOT/src, misc formatting.

Nice side-effect: almost 3% faster runs because it's much faster to compute
line number differences in the generated output than the incoming source.

Benchmark run, best of 5 runs, before and after:
BenchmarkPrint	     200	  12347587 ns/op
BenchmarkPrint	     200	  11999061 ns/op

Fixes #4504.

LGTM=adonovan
R=golang-codereviews, adonovan
CC=golang-codereviews
https://golang.org/cl/69260045
parent b00e4770
...@@ -378,10 +378,6 @@ func (p *printer) setLineComment(text string) { ...@@ -378,10 +378,6 @@ func (p *printer) setLineComment(text string) {
p.setComment(&ast.CommentGroup{List: []*ast.Comment{{Slash: token.NoPos, Text: text}}}) p.setComment(&ast.CommentGroup{List: []*ast.Comment{{Slash: token.NoPos, Text: text}}})
} }
func (p *printer) isMultiLine(n ast.Node) bool {
return p.lineFor(n.End())-p.lineFor(n.Pos()) > 0
}
func (p *printer) fieldList(fields *ast.FieldList, isStruct, isIncomplete bool) { func (p *printer) fieldList(fields *ast.FieldList, isStruct, isIncomplete bool) {
lbrace := fields.Opening lbrace := fields.Opening
list := fields.List list := fields.List
...@@ -428,13 +424,14 @@ func (p *printer) fieldList(fields *ast.FieldList, isStruct, isIncomplete bool) ...@@ -428,13 +424,14 @@ func (p *printer) fieldList(fields *ast.FieldList, isStruct, isIncomplete bool)
if len(list) == 1 { if len(list) == 1 {
sep = blank sep = blank
} }
newSection := false var line int
for i, f := range list { for i, f := range list {
if i > 0 { if i > 0 {
p.linebreak(p.lineFor(f.Pos()), 1, ignore, newSection) p.linebreak(p.lineFor(f.Pos()), 1, ignore, p.linesFrom(line) > 0)
} }
extraTabs := 0 extraTabs := 0
p.setComment(f.Doc) p.setComment(f.Doc)
p.recordLine(&line)
if len(f.Names) > 0 { if len(f.Names) > 0 {
// named fields // named fields
p.identList(f.Names, false) p.identList(f.Names, false)
...@@ -460,7 +457,6 @@ func (p *printer) fieldList(fields *ast.FieldList, isStruct, isIncomplete bool) ...@@ -460,7 +457,6 @@ func (p *printer) fieldList(fields *ast.FieldList, isStruct, isIncomplete bool)
} }
p.setComment(f.Comment) p.setComment(f.Comment)
} }
newSection = p.isMultiLine(f)
} }
if isIncomplete { if isIncomplete {
if len(list) > 0 { if len(list) > 0 {
...@@ -472,12 +468,13 @@ func (p *printer) fieldList(fields *ast.FieldList, isStruct, isIncomplete bool) ...@@ -472,12 +468,13 @@ func (p *printer) fieldList(fields *ast.FieldList, isStruct, isIncomplete bool)
} else { // interface } else { // interface
newSection := false var line int
for i, f := range list { for i, f := range list {
if i > 0 { if i > 0 {
p.linebreak(p.lineFor(f.Pos()), 1, ignore, newSection) p.linebreak(p.lineFor(f.Pos()), 1, ignore, p.linesFrom(line) > 0)
} }
p.setComment(f.Doc) p.setComment(f.Doc)
p.recordLine(&line)
if ftyp, isFtyp := f.Type.(*ast.FuncType); isFtyp { if ftyp, isFtyp := f.Type.(*ast.FuncType); isFtyp {
// method // method
p.expr(f.Names[0]) p.expr(f.Names[0])
...@@ -487,7 +484,6 @@ func (p *printer) fieldList(fields *ast.FieldList, isStruct, isIncomplete bool) ...@@ -487,7 +484,6 @@ func (p *printer) fieldList(fields *ast.FieldList, isStruct, isIncomplete bool)
p.expr(f.Type) p.expr(f.Type)
} }
p.setComment(f.Comment) p.setComment(f.Comment)
newSection = p.isMultiLine(f)
} }
if isIncomplete { if isIncomplete {
if len(list) > 0 { if len(list) > 0 {
...@@ -907,7 +903,7 @@ func (p *printer) stmtList(list []ast.Stmt, nindent int, nextIsRBrace bool) { ...@@ -907,7 +903,7 @@ func (p *printer) stmtList(list []ast.Stmt, nindent int, nextIsRBrace bool) {
if nindent > 0 { if nindent > 0 {
p.print(indent) p.print(indent)
} }
multiLine := false var line int
i := 0 i := 0
for _, s := range list { for _, s := range list {
// ignore empty statements (was issue 3466) // ignore empty statements (was issue 3466)
...@@ -917,14 +913,21 @@ func (p *printer) stmtList(list []ast.Stmt, nindent int, nextIsRBrace bool) { ...@@ -917,14 +913,21 @@ func (p *printer) stmtList(list []ast.Stmt, nindent int, nextIsRBrace bool) {
if len(p.output) > 0 { if len(p.output) > 0 {
// only print line break if we are not at the beginning of the output // only print line break if we are not at the beginning of the output
// (i.e., we are not printing only a partial program) // (i.e., we are not printing only a partial program)
p.linebreak(p.lineFor(s.Pos()), 1, ignore, i == 0 || nindent == 0 || multiLine) p.linebreak(p.lineFor(s.Pos()), 1, ignore, i == 0 || nindent == 0 || p.linesFrom(line) > 0)
} }
p.recordLine(&line)
p.stmt(s, nextIsRBrace && i == len(list)-1) p.stmt(s, nextIsRBrace && i == len(list)-1)
// labeled statements put labels on a separate line, but here // labeled statements put labels on a separate line, but here
// we only care about whether the actual statement w/o label // we only care about the start line of the actual statement
// is a multi-line statement - remove the label first // without label - correct line for each label
// (was issue 5623) for t := s; ; {
multiLine = p.isMultiLine(unlabeledStmt(s)) lt, _ := t.(*ast.LabeledStmt)
if lt == nil {
break
}
line++
t = lt.Stmt
}
i++ i++
} }
} }
...@@ -933,15 +936,6 @@ func (p *printer) stmtList(list []ast.Stmt, nindent int, nextIsRBrace bool) { ...@@ -933,15 +936,6 @@ func (p *printer) stmtList(list []ast.Stmt, nindent int, nextIsRBrace bool) {
} }
} }
// unlabeledStmt returns the statement of a labeled statement s;
// otherwise it return s.
func unlabeledStmt(s ast.Stmt) ast.Stmt {
if s, _ := s.(*ast.LabeledStmt); s != nil {
return unlabeledStmt(s.Stmt)
}
return s
}
// block prints an *ast.BlockStmt; it always spans at least two lines. // block prints an *ast.BlockStmt; it always spans at least two lines.
func (p *printer) block(b *ast.BlockStmt, nindent int) { func (p *printer) block(b *ast.BlockStmt, nindent int) {
p.print(b.Lbrace, token.LBRACE) p.print(b.Lbrace, token.LBRACE)
...@@ -1394,22 +1388,22 @@ func (p *printer) genDecl(d *ast.GenDecl) { ...@@ -1394,22 +1388,22 @@ func (p *printer) genDecl(d *ast.GenDecl) {
// two or more grouped const/var declarations: // two or more grouped const/var declarations:
// determine if the type column must be kept // determine if the type column must be kept
keepType := keepTypeColumn(d.Specs) keepType := keepTypeColumn(d.Specs)
newSection := false var line int
for i, s := range d.Specs { for i, s := range d.Specs {
if i > 0 { if i > 0 {
p.linebreak(p.lineFor(s.Pos()), 1, ignore, newSection) p.linebreak(p.lineFor(s.Pos()), 1, ignore, p.linesFrom(line) > 0)
} }
p.recordLine(&line)
p.valueSpec(s.(*ast.ValueSpec), keepType[i]) p.valueSpec(s.(*ast.ValueSpec), keepType[i])
newSection = p.isMultiLine(s)
} }
} else { } else {
newSection := false var line int
for i, s := range d.Specs { for i, s := range d.Specs {
if i > 0 { if i > 0 {
p.linebreak(p.lineFor(s.Pos()), 1, ignore, newSection) p.linebreak(p.lineFor(s.Pos()), 1, ignore, p.linesFrom(line) > 0)
} }
p.recordLine(&line)
p.spec(s, n, false) p.spec(s, n, false)
newSection = p.isMultiLine(s)
} }
} }
p.print(unindent, formfeed) p.print(unindent, formfeed)
......
...@@ -73,6 +73,7 @@ type printer struct { ...@@ -73,6 +73,7 @@ type printer struct {
pos token.Position // current position in AST (source) space pos token.Position // current position in AST (source) space
out token.Position // current position in output space out token.Position // current position in output space
last token.Position // value of pos after calling writeString last token.Position // value of pos after calling writeString
linePtr *int // if set, record out.Line for the next token in *linePtr
// The list of all source comments, in order of appearance. // The list of all source comments, in order of appearance.
comments []*ast.CommentGroup // may be nil comments []*ast.CommentGroup // may be nil
...@@ -99,6 +100,14 @@ func (p *printer) init(cfg *Config, fset *token.FileSet, nodeSizes map[ast.Node] ...@@ -99,6 +100,14 @@ func (p *printer) init(cfg *Config, fset *token.FileSet, nodeSizes map[ast.Node]
p.cachedPos = -1 p.cachedPos = -1
} }
func (p *printer) internalError(msg ...interface{}) {
if debug {
fmt.Print(p.pos.String() + ": ")
fmt.Println(msg...)
panic("go/printer")
}
}
// commentsHaveNewline reports whether a list of comments belonging to // commentsHaveNewline reports whether a list of comments belonging to
// an *ast.CommentGroup contains newlines. Because the position information // an *ast.CommentGroup contains newlines. Because the position information
// may only be partially correct, we also have to read the comment text. // may only be partially correct, we also have to read the comment text.
...@@ -162,12 +171,22 @@ func (p *printer) commentSizeBefore(next token.Position) int { ...@@ -162,12 +171,22 @@ func (p *printer) commentSizeBefore(next token.Position) int {
return size return size
} }
func (p *printer) internalError(msg ...interface{}) { // recordLine records the output line number for the next non-whitespace
if debug { // token in *linePtr. It is used to compute an accurate line number for a
fmt.Print(p.pos.String() + ": ") // formatted construct, independent of pending (not yet emitted) whitespace
fmt.Println(msg...) // or comments.
panic("go/printer") //
} func (p *printer) recordLine(linePtr *int) {
p.linePtr = linePtr
}
// linesFrom returns the number of output lines between the current
// output line and the line argument, ignoring any pending (not yet
// emitted) whitespace or comments. It is used to compute an accurate
// size (in number of lines) for a formatted construct.
//
func (p *printer) linesFrom(line int) int {
return p.out.Line - line
} }
func (p *printer) posFor(pos token.Pos) token.Position { func (p *printer) posFor(pos token.Pos) token.Position {
...@@ -943,6 +962,12 @@ func (p *printer) print(args ...interface{}) { ...@@ -943,6 +962,12 @@ func (p *printer) print(args ...interface{}) {
} }
} }
// the next token starts now - record its line number if requested
if p.linePtr != nil {
*p.linePtr = p.out.Line
p.linePtr = nil
}
p.writeString(next, data, isLit) p.writeString(next, data, isLit)
p.impliedSemi = impliedSemi p.impliedSemi = impliedSemi
} }
......
...@@ -91,6 +91,7 @@ LLLLLLL: ...@@ -91,6 +91,7 @@ LLLLLLL:
_ = xxxxxxxxxxxxxxxxxxxxxxxxxxxx // comment _ = xxxxxxxxxxxxxxxxxxxxxxxxxxxx // comment
LL: LL:
LLLLL:
_ = xxxxxxxxxxxxxxxxxxxxxxxxxxxx /* comment */ _ = xxxxxxxxxxxxxxxxxxxxxxxxxxxx /* comment */
_ = yyyyyyyyyyyyyyyy /* comment - should be aligned */ _ = yyyyyyyyyyyyyyyy /* comment - should be aligned */
......
...@@ -91,6 +91,7 @@ LLLLLLL: ...@@ -91,6 +91,7 @@ LLLLLLL:
_ = xxxxxxxxxxxxxxxxxxxxxxxxxxxx // comment _ = xxxxxxxxxxxxxxxxxxxxxxxxxxxx // comment
LL: LL:
LLLLL:
_ = xxxxxxxxxxxxxxxxxxxxxxxxxxxx /* comment */ _ = xxxxxxxxxxxxxxxxxxxxxxxxxxxx /* comment */
_ = yyyyyyyyyyyyyyyy /* comment - should be aligned */ _ = yyyyyyyyyyyyyyyy /* comment - should be aligned */
......
...@@ -397,6 +397,21 @@ func _() { ...@@ -397,6 +397,21 @@ func _() {
} }
} }
// use the formatted output rather than the input to decide when to align
// (was issue 4505)
const (
short = 2 * (1 + 2)
aMuchLongerName = 3
)
var (
short = X{}
aMuchLongerName = X{}
x1 = X{} // foo
x2 = X{} // foo
)
func _() { func _() {
type ( type (
xxxxxx int xxxxxx int
......
...@@ -409,6 +409,24 @@ func _() { ...@@ -409,6 +409,24 @@ func _() {
} }
} }
// use the formatted output rather than the input to decide when to align
// (was issue 4505)
const (
short = 2 * (
1 + 2)
aMuchLongerName = 3
)
var (
short = X{
}
aMuchLongerName = X{}
x1 = X{} // foo
x2 = X{
} // foo
)
func _() { func _() {
type ( type (
xxxxxx int xxxxxx int
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment