]> Cypherpunks repositories - gostls13.git/commitdiff
go/printer: don't lose relevant parentheses when rewriting selector expressions
authorRobert Griesemer <gri@golang.org>
Wed, 15 Feb 2012 20:25:37 +0000 (12:25 -0800)
committerRobert Griesemer <gri@golang.org>
Wed, 15 Feb 2012 20:25:37 +0000 (12:25 -0800)
Also: Simplified handling of selector expressions. As a result, complicated
multi-line expressions containing selectors and calls/indices with arguments
broken accross lines don't get indented the same way as before, but the change
is minimal (see tests) and there's no such code in the std library. It seems
a worthwhile compromise given the much simpler code.

Applied gofmt -w $GOROOT/src $GOROOT/misc .

Fixes #1847.

R=rsc
CC=golang-dev
https://golang.org/cl/5675062

misc/dashboard/builder/main.go
src/cmd/gofmt/gofmt_test.go
src/cmd/gofmt/testdata/rewrite4.golden [new file with mode: 0644]
src/cmd/gofmt/testdata/rewrite4.input [new file with mode: 0644]
src/pkg/go/printer/nodes.go
src/pkg/go/printer/testdata/expressions.golden
src/pkg/go/printer/testdata/expressions.raw

index 84f44a3b0a991f4eeb5b29edd1c0d5722a7e86f7..7ca627670b5e301e8f06d983a701944f1808b301 100644 (file)
@@ -494,7 +494,7 @@ func (b *Builder) envvWindows() []string {
                "GOOS":         b.goos,
                "GOARCH":       b.goarch,
                "GOROOT_FINAL": `c:\go`,
-               "GOBUILDEXIT": "1", // exit all.bat with completion status.
+               "GOBUILDEXIT":  "1", // exit all.bat with completion status.
        }
        for _, name := range extraEnv {
                s, err := os.Getenverror(name)
index 303c4f1e1c2abb290505560dd3cd1294dc47850e..4b280500976a29c9848fb7df3e65428ca9cfba02 100644 (file)
@@ -77,6 +77,7 @@ var tests = []struct {
        {"testdata/rewrite1.input", "-r=Foo->Bar"},
        {"testdata/rewrite2.input", "-r=int->bool"},
        {"testdata/rewrite3.input", "-r=x->x"},
+       {"testdata/rewrite4.input", "-r=(x)->x"},
        {"testdata/stdin*.input", "-stdin"},
        {"testdata/comments.input", ""},
        {"testdata/import.input", ""},
diff --git a/src/cmd/gofmt/testdata/rewrite4.golden b/src/cmd/gofmt/testdata/rewrite4.golden
new file mode 100644 (file)
index 0000000..8dfc81a
--- /dev/null
@@ -0,0 +1,74 @@
+// Copyright 2012 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Rewriting of parenthesized expressions (x) -> x
+// must not drop parentheses if that would lead to
+// wrong association of the operands.
+// Was issue 1847.
+
+package main
+
+// From example 1 of issue 1847.
+func _() {
+       var t = (&T{1000}).Id()
+}
+
+// From example 2 of issue 1847.
+func _() {
+       fmt.Println((*xpp).a)
+}
+
+// Some more test cases.
+func _() {
+       _ = (-x).f
+       _ = (*x).f
+       _ = (&x).f
+       _ = (!x).f
+       _ = -x.f
+       _ = *x.f
+       _ = &x.f
+       _ = !x.f
+       (-x).f()
+       (*x).f()
+       (&x).f()
+       (!x).f()
+       _ = -x.f()
+       _ = *x.f()
+       _ = &x.f()
+       _ = !x.f()
+
+       _ = (-x).f
+       _ = (*x).f
+       _ = (&x).f
+       _ = (!x).f
+       _ = -x.f
+       _ = *x.f
+       _ = &x.f
+       _ = !x.f
+       (-x).f()
+       (*x).f()
+       (&x).f()
+       (!x).f()
+       _ = -x.f()
+       _ = *x.f()
+       _ = &x.f()
+       _ = !x.f()
+
+       _ = -x.f
+       _ = *x.f
+       _ = &x.f
+       _ = !x.f
+       _ = -x.f
+       _ = *x.f
+       _ = &x.f
+       _ = !x.f
+       _ = -x.f()
+       _ = *x.f()
+       _ = &x.f()
+       _ = !x.f()
+       _ = -x.f()
+       _ = *x.f()
+       _ = &x.f()
+       _ = !x.f()
+}
diff --git a/src/cmd/gofmt/testdata/rewrite4.input b/src/cmd/gofmt/testdata/rewrite4.input
new file mode 100644 (file)
index 0000000..164cc04
--- /dev/null
@@ -0,0 +1,74 @@
+// Copyright 2012 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// Rewriting of parenthesized expressions (x) -> x
+// must not drop parentheses if that would lead to
+// wrong association of the operands.
+// Was issue 1847.
+
+package main
+
+// From example 1 of issue 1847.
+func _() {
+       var t = (&T{1000}).Id()
+}
+
+// From example 2 of issue 1847.
+func _() {
+       fmt.Println((*xpp).a)
+}
+
+// Some more test cases.
+func _() {
+       _ = (-x).f
+       _ = (*x).f
+       _ = (&x).f
+       _ = (!x).f
+       _ = (-x.f)
+       _ = (*x.f)
+       _ = (&x.f)
+       _ = (!x.f)
+       (-x).f()
+       (*x).f()
+       (&x).f()
+       (!x).f()
+       _ = (-x.f())
+       _ = (*x.f())
+       _ = (&x.f())
+       _ = (!x.f())
+
+       _ = ((-x)).f
+       _ = ((*x)).f
+       _ = ((&x)).f
+       _ = ((!x)).f
+       _ = ((-x.f))
+       _ = ((*x.f))
+       _ = ((&x.f))
+       _ = ((!x.f))
+       ((-x)).f()
+       ((*x)).f()
+       ((&x)).f()
+       ((!x)).f()
+       _ = ((-x.f()))
+       _ = ((*x.f()))
+       _ = ((&x.f()))
+       _ = ((!x.f()))
+
+       _ = -(x).f
+       _ = *(x).f
+       _ = &(x).f
+       _ = !(x).f
+       _ = -x.f
+       _ = *x.f
+       _ = &x.f
+       _ = !x.f
+       _ = -(x).f()
+       _ = *(x).f()
+       _ = &(x).f()
+       _ = !(x).f()
+       _ = -x.f()
+       _ = *x.f()
+       _ = &x.f()
+       _ = !x.f()
+}
index 25935fb42bb8b369be3791b28c2cb83224f0b5c0..b095f508daab5b7bf7030efd12d114ce9992bb99 100644 (file)
@@ -87,7 +87,6 @@ const (
        commaSep                            // elements are separated by commas
        commaTerm                           // list is optionally terminated by a comma
        noIndent                            // no extra indentation in multi-line lists
-       periodSep                           // elements are separated by periods
 )
 
 // Sets multiLine to true if the identifier list spans multiple lines.
@@ -213,13 +212,10 @@ func (p *printer) exprList(prev0 token.Pos, list []ast.Expr, depth int, mode exp
                }
 
                if i > 0 {
-                       switch {
-                       case mode&commaSep != 0:
+                       if mode&commaSep != 0 {
                                p.print(token.COMMA)
-                       case mode&periodSep != 0:
-                               p.print(token.PERIOD)
                        }
-                       needsBlank := mode&periodSep == 0 // period-separated list elements don't need a blank
+                       needsBlank := true
                        if prevLine < line && prevLine > 0 && line > 0 {
                                // lines are broken using newlines so comments remain aligned
                                // unless forceFF is set or there are multiple expressions on
@@ -668,63 +664,6 @@ func isBinary(expr ast.Expr) bool {
        return ok
 }
 
-// If the expression contains one or more selector expressions, splits it into
-// two expressions at the rightmost period. Writes entire expr to suffix when
-// selector isn't found. Rewrites AST nodes for calls, index expressions and
-// type assertions, all of which may be found in selector chains, to make them
-// parts of the chain.
-func splitSelector(expr ast.Expr) (body, suffix ast.Expr) {
-       switch x := expr.(type) {
-       case *ast.SelectorExpr:
-               body, suffix = x.X, x.Sel
-               return
-       case *ast.CallExpr:
-               body, suffix = splitSelector(x.Fun)
-               if body != nil {
-                       suffix = &ast.CallExpr{suffix, x.Lparen, x.Args, x.Ellipsis, x.Rparen}
-                       return
-               }
-       case *ast.IndexExpr:
-               body, suffix = splitSelector(x.X)
-               if body != nil {
-                       suffix = &ast.IndexExpr{suffix, x.Lbrack, x.Index, x.Rbrack}
-                       return
-               }
-       case *ast.SliceExpr:
-               body, suffix = splitSelector(x.X)
-               if body != nil {
-                       suffix = &ast.SliceExpr{suffix, x.Lbrack, x.Low, x.High, x.Rbrack}
-                       return
-               }
-       case *ast.TypeAssertExpr:
-               body, suffix = splitSelector(x.X)
-               if body != nil {
-                       suffix = &ast.TypeAssertExpr{suffix, x.Type}
-                       return
-               }
-       }
-       suffix = expr
-       return
-}
-
-// Convert an expression into an expression list split at the periods of
-// selector expressions.
-func selectorExprList(expr ast.Expr) (list []ast.Expr) {
-       // split expression
-       for expr != nil {
-               var suffix ast.Expr
-               expr, suffix = splitSelector(expr)
-               list = append(list, suffix)
-       }
-
-       // reverse list
-       for i, j := 0, len(list)-1; i < j; i, j = i+1, j-1 {
-               list[i], list[j] = list[j], list[i]
-       }
-
-       return
-}
-
 // Sets multiLine to true if the expression spans multiple lines.
 func (p *printer) expr1(expr ast.Expr, prec1, depth int, multiLine *bool) {
        p.print(expr.Pos())
@@ -798,8 +737,14 @@ func (p *printer) expr1(expr ast.Expr, prec1, depth int, multiLine *bool) {
                }
 
        case *ast.SelectorExpr:
-               parts := selectorExprList(expr)
-               p.exprList(token.NoPos, parts, depth, periodSep, multiLine, token.NoPos)
+               p.expr1(x.X, token.HighestPrec, depth, multiLine)
+               p.print(token.PERIOD)
+               if line := p.lineFor(x.Sel.Pos()); p.pos.IsValid() && p.pos.Line < line {
+                       p.print(indent, newline, x.Sel.Pos(), x.Sel, unindent)
+                       *multiLine = true
+               } else {
+                       p.print(x.Sel.Pos(), x.Sel)
+               }
 
        case *ast.TypeAssertExpr:
                p.expr1(x.X, token.HighestPrec, depth, multiLine)
index d0cf24ad6f6d8bd968f4d51837d41a2a311eaa95..95fdd95ffbba598e94b986fda2684286ba42eeb6 100644 (file)
@@ -545,7 +545,7 @@ func _() {
        // handle multiline argument list correctly
        _ = new(T).
                foo(
-                       1).
+               1).
                foo(2)
 
        _ = new(T).foo(
@@ -587,12 +587,12 @@ func _() {
        _ = new(T).
                Field.
                Array[3+
-                       4].
+               4].
                Table["foo"].
                Blob.(*Type).
                Slices[1:4].
                Method(1, 2,
-                       3).
+               3).
                Thingy
 
        _ = a.b.c
index d7819a3baadcff99f25bfe2f4e468c78a37fbb5f..3442ba9b9501da9e83b2c900ce03def1bfecbe74 100644 (file)
@@ -545,7 +545,7 @@ func _() {
        // handle multiline argument list correctly
        _ = new(T).
                foo(
-                       1).
+               1).
                foo(2)
 
        _ = new(T).foo(
@@ -587,12 +587,12 @@ func _() {
        _ = new(T).
                Field.
                Array[3+
-                       4].
+               4].
                Table["foo"].
                Blob.(*Type).
                Slices[1:4].
                Method(1, 2,
-                       3).
+               3).
                Thingy
 
        _ = a.b.c