code review 70190046: go/parser: better error position for non-invoked gp/def...

go/parser: better error position for non-invoked gp/defer functions

Added test cases and expanded test harness to handle token end
positions.

Also: Make sure token end positions are never outside the valid
      position range, as was possible in case of parse errors.

Fixes issue 7458.

[gri, 2014-03-04 22:01:34]

Hello bradfitz@golang.org (cc: golang-codereviews@googlegroups.com),

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

[bradfitz, 2014-03-04 22:06:45]

LGTM

Thanks!

https://codereview.appspot.com/70190046/diff/60001/src/pkg/go/parser/error_te...
File src/pkg/go/parser/error_test.go (right):

https://codereview.appspot.com/70190046/diff/60001/src/pkg/go/parser/error_te...
src/pkg/go/parser/error_test.go:66: var errRx = regexp.MustCompile(`^/\* *ERROR
*(HERE|) *"([^"]*)" *\*/$`)
or (HERE)?

https://codereview.appspot.com/70190046/diff/60001/src/pkg/go/parser/parser.go
File src/pkg/go/parser/parser.go (right):

https://codereview.appspot.com/70190046/diff/60001/src/pkg/go/parser/parser.g...
src/pkg/go/parser/parser.go:1709: func (p *parser) parseCallExpr(s string)
*ast.CallExpr {
I'd document s here, and/or rename it. naively, it looks like this method parses
s, but really it's either the string "go" or "defer"... maybe a callType string.

[gri, 2014-03-04 22:10:30]

https://codereview.appspot.com/70190046/diff/60001/src/pkg/go/parser/error_te...
File src/pkg/go/parser/error_test.go (right):

https://codereview.appspot.com/70190046/diff/60001/src/pkg/go/parser/error_te...
src/pkg/go/parser/error_test.go:66: var errRx = regexp.MustCompile(`^/\* *ERROR
*(HERE|) *"([^"]*)" *\*/$`)
On 2014/03/04 22:06:46, bradfitz wrote:
> or (HERE)?

Done.

https://codereview.appspot.com/70190046/diff/60001/src/pkg/go/parser/parser.go
File src/pkg/go/parser/parser.go (right):

https://codereview.appspot.com/70190046/diff/60001/src/pkg/go/parser/parser.g...
src/pkg/go/parser/parser.go:1709: func (p *parser) parseCallExpr(s string)
*ast.CallExpr {
On 2014/03/04 22:06:46, bradfitz wrote:
> I'd document s here, and/or rename it. naively, it looks like this method
parses
> s, but really it's either the string "go" or "defer"... maybe a callType
string.

Done.

[gri, 2014-03-04 22:10:41]

*** Submitted as https://code.google.com/p/go/source/detail?r=72b9500c86ce ***

go/parser: better error position for non-invoked gp/defer functions

Added test cases and expanded test harness to handle token end
positions.

Also: Make sure token end positions are never outside the valid
      position range, as was possible in case of parse errors.

Fixes issue 7458.

LGTM=bradfitz
R=bradfitz
CC=golang-codereviews
https://codereview.appspot.com/70190046

[gobot, 2014-03-04 22:40:37]

This CL appears to have broken the windows-386-ec2 builder.