FAQ
Reviewers: iant,

Message:
Hello iant@golang.org (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go


Description:
go/ast: track position of <- for channel types

This is a backward-compatible API change.

Without the correct <- position information,
certain channel types have incorrect position
information.

Please review this at http://codereview.appspot.com/6585063/

Affected files:
M src/pkg/go/ast/ast.go
M src/pkg/go/parser/parser.go
M src/pkg/go/parser/short_test.go
M src/pkg/go/printer/nodes.go


Index: src/pkg/go/ast/ast.go
===================================================================
--- a/src/pkg/go/ast/ast.go
+++ b/src/pkg/go/ast/ast.go
@@ -407,6 +407,7 @@
// A ChanType node represents a channel type.
ChanType struct {
Begin token.Pos // position of "chan" keyword or "<-" (whichever comes
first)
+ Arrow token.Pos // position of "<-" (noPos if there is no "<-")
Dir ChanDir // channel direction
Value Expr // value type
}
Index: src/pkg/go/parser/parser.go
===================================================================
--- a/src/pkg/go/parser/parser.go
+++ b/src/pkg/go/parser/parser.go
@@ -924,20 +924,22 @@

pos := p.pos
dir := ast.SEND | ast.RECV
+ var arrow token.Pos
if p.tok == token.CHAN {
p.next()
if p.tok == token.ARROW {
+ arrow = p.pos
p.next()
dir = ast.SEND
}
} else {
- p.expect(token.ARROW)
+ arrow = p.expect(token.ARROW)
p.expect(token.CHAN)
dir = ast.RECV
}
value := p.parseType()

- return &ast.ChanType{Begin: pos, Dir: dir, Value: value}
+ return &ast.ChanType{Begin: pos, Arrow: arrow, Dir: dir, Value: value}
}

// If the result is an identifier, it is not resolved.
@@ -1397,7 +1399,7 @@

case token.ARROW:
// channel type or receive expression
- pos := p.pos
+ arrow := p.pos
p.next()

// If the next token is token.CHAN we still don't know if it
@@ -1421,29 +1423,25 @@
// (<-type)

// re-associate position info and <-
- arrow := true
- for ok && arrow {
- begin := typ.Begin
+ dir := ast.SEND
+ for ok && dir == ast.SEND {
if typ.Dir == ast.RECV {
// error: (<-type) is (<-(<-chan T))
- p.errorExpected(begin, "'chan'")
+ p.errorExpected(typ.Arrow, "'chan'")
}
- arrow = typ.Dir == ast.SEND
- typ.Begin = pos
- typ.Dir = ast.RECV
+ arrow, typ.Begin, typ.Arrow = typ.Arrow, arrow, arrow
+ dir, typ.Dir = typ.Dir, ast.RECV
typ, ok = typ.Value.(*ast.ChanType)
- // TODO(gri) ast.ChanType should store exact <- position
- pos = begin // estimate (we don't have the exact position of <- for
send channels)
}
- if arrow {
- p.errorExpected(pos, "'chan'")
+ if dir == ast.SEND {
+ p.errorExpected(arrow, "channel type")
}

return x
}

// <-(expr)
- return &ast.UnaryExpr{OpPos: pos, Op: token.ARROW, X: p.checkExpr(x)}
+ return &ast.UnaryExpr{OpPos: arrow, Op: token.ARROW, X: p.checkExpr(x)}

case token.MUL:
// pointer type or unary "*" expression
Index: src/pkg/go/parser/short_test.go
===================================================================
--- a/src/pkg/go/parser/short_test.go
+++ b/src/pkg/go/parser/short_test.go
@@ -69,7 +69,7 @@
`package p; var a = <- /* ERROR "expected expression" */ chan int;`,
`package p; func f() { select { case _ <- chan /* ERROR "expected
expression" */ int: } };`,
`package p; func f() { _ = (<-<- /* ERROR "expected 'chan'" */ chan
int)(nil) };`,
- `package p; func f() { _ = (<-chan<-chan<-chan<-chan<-chan /*
ERROR "expected 'chan'" */ <-int)(nil) };`,
+ `package p; func f() { _ = (<-chan<-chan<-chan<-chan<-chan<- /*
ERROR "expected channel type" */ int)(nil) };`,
}

func TestInvalid(t *testing.T) {
Index: src/pkg/go/printer/nodes.go
===================================================================
--- a/src/pkg/go/printer/nodes.go
+++ b/src/pkg/go/printer/nodes.go
@@ -853,9 +853,9 @@
case ast.SEND | ast.RECV:
p.print(token.CHAN)
case ast.RECV:
- p.print(token.ARROW, token.CHAN)
+ p.print(token.ARROW, token.CHAN) // x.Arrow and x.Pos() are the same
case ast.SEND:
- p.print(token.CHAN, token.ARROW)
+ p.print(token.CHAN, x.Arrow, token.ARROW)
}
p.print(blank)
p.expr(x.Value)

Search Discussions

  • Ian Lance Taylor at Oct 3, 2012 at 12:47 am
    LGTM
    On Tue, Oct 2, 2012 at 5:38 PM, wrote:
    Reviewers: iant,

    Message:
    Hello iant@golang.org (cc: golang-dev@googlegroups.com),

    I'd like you to review this change to
    https://code.google.com/p/go


    Description:
    go/ast: track position of <- for channel types

    This is a backward-compatible API change.

    Without the correct <- position information,
    certain channel types have incorrect position
    information.

    Please review this at http://codereview.appspot.com/6585063/

    Affected files:
    M src/pkg/go/ast/ast.go
    M src/pkg/go/parser/parser.go
    M src/pkg/go/parser/short_test.go
    M src/pkg/go/printer/nodes.go


    Index: src/pkg/go/ast/ast.go
    ===================================================================
    --- a/src/pkg/go/ast/ast.go
    +++ b/src/pkg/go/ast/ast.go
    @@ -407,6 +407,7 @@
    // A ChanType node represents a channel type.
    ChanType struct {
    Begin token.Pos // position of "chan" keyword or "<-"
    (whichever comes first)
    + Arrow token.Pos // position of "<-" (noPos if there is no
    "<-")
    Dir ChanDir // channel direction
    Value Expr // value type
    }
    Index: src/pkg/go/parser/parser.go
    ===================================================================
    --- a/src/pkg/go/parser/parser.go
    +++ b/src/pkg/go/parser/parser.go
    @@ -924,20 +924,22 @@

    pos := p.pos
    dir := ast.SEND | ast.RECV
    + var arrow token.Pos
    if p.tok == token.CHAN {
    p.next()
    if p.tok == token.ARROW {
    + arrow = p.pos
    p.next()
    dir = ast.SEND
    }
    } else {
    - p.expect(token.ARROW)
    + arrow = p.expect(token.ARROW)
    p.expect(token.CHAN)
    dir = ast.RECV
    }
    value := p.parseType()

    - return &ast.ChanType{Begin: pos, Dir: dir, Value: value}
    + return &ast.ChanType{Begin: pos, Arrow: arrow, Dir: dir, Value:
    value}
    }

    // If the result is an identifier, it is not resolved.
    @@ -1397,7 +1399,7 @@

    case token.ARROW:
    // channel type or receive expression
    - pos := p.pos
    + arrow := p.pos
    p.next()

    // If the next token is token.CHAN we still don't know if it
    @@ -1421,29 +1423,25 @@
    // (<-type)

    // re-associate position info and <-
    - arrow := true
    - for ok && arrow {
    - begin := typ.Begin
    + dir := ast.SEND
    + for ok && dir == ast.SEND {
    if typ.Dir == ast.RECV {
    // error: (<-type) is (<-(<-chan T))
    - p.errorExpected(begin, "'chan'")
    + p.errorExpected(typ.Arrow, "'chan'")
    }
    - arrow = typ.Dir == ast.SEND
    - typ.Begin = pos
    - typ.Dir = ast.RECV
    + arrow, typ.Begin, typ.Arrow = typ.Arrow,
    arrow, arrow
    + dir, typ.Dir = typ.Dir, ast.RECV
    typ, ok = typ.Value.(*ast.ChanType)
    - // TODO(gri) ast.ChanType should store exact
    <- position
    - pos = begin // estimate (we don't have the
    exact position of <- for send channels)
    }
    - if arrow {
    - p.errorExpected(pos, "'chan'")
    + if dir == ast.SEND {
    + p.errorExpected(arrow, "channel type")
    }

    return x
    }

    // <-(expr)
    - return &ast.UnaryExpr{OpPos: pos, Op: token.ARROW, X:
    p.checkExpr(x)}
    + return &ast.UnaryExpr{OpPos: arrow, Op: token.ARROW, X:
    p.checkExpr(x)}

    case token.MUL:
    // pointer type or unary "*" expression
    Index: src/pkg/go/parser/short_test.go
    ===================================================================
    --- a/src/pkg/go/parser/short_test.go
    +++ b/src/pkg/go/parser/short_test.go
    @@ -69,7 +69,7 @@
    `package p; var a = <- /* ERROR "expected expression" */ chan int;`,
    `package p; func f() { select { case _ <- chan /* ERROR "expected
    expression" */ int: } };`,
    `package p; func f() { _ = (<-<- /* ERROR "expected 'chan'" */ chan
    int)(nil) };`,
    - `package p; func f() { _ = (<-chan<-chan<-chan<-chan<-chan /* ERROR
    "expected 'chan'" */ <-int)(nil) };`,
    + `package p; func f() { _ = (<-chan<-chan<-chan<-chan<-chan<- /*
    ERROR "expected channel type" */ int)(nil) };`,
    }

    func TestInvalid(t *testing.T) {
    Index: src/pkg/go/printer/nodes.go
    ===================================================================
    --- a/src/pkg/go/printer/nodes.go
    +++ b/src/pkg/go/printer/nodes.go
    @@ -853,9 +853,9 @@
    case ast.SEND | ast.RECV:
    p.print(token.CHAN)
    case ast.RECV:
    - p.print(token.ARROW, token.CHAN)
    + p.print(token.ARROW, token.CHAN) // x.Arrow and
    x.Pos() are the same
    case ast.SEND:
    - p.print(token.CHAN, token.ARROW)
    + p.print(token.CHAN, x.Arrow, token.ARROW)
    }
    p.print(blank)
    p.expr(x.Value)
  • Gri at Oct 3, 2012 at 12:56 am
    *** Submitted as
    http://code.google.com/p/go/source/detail?r=3ae6c87de7f2 ***

    go/ast: track position of <- for channel types

    This is a backward-compatible API change.

    Without the correct <- position information,
    certain channel types have incorrect position
    information.

    R=iant, iant
    CC=golang-dev
    http://codereview.appspot.com/6585063


    http://codereview.appspot.com/6585063/

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupgolang-dev @
categoriesgo
postedOct 3, '12 at 12:39a
activeOct 3, '12 at 12:56a
posts3
users2
websitegolang.org

2 users in discussion

Gri: 2 posts Ian Lance Taylor: 1 post

People

Translate

site design / logo © 2022 Grokbase