Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

go/parser: EmptyStmt.Semicolon.End() is off by one #9979

Closed
rsc opened this issue Feb 24, 2015 · 1 comment
Closed

go/parser: EmptyStmt.Semicolon.End() is off by one #9979

rsc opened this issue Feb 24, 2015 · 1 comment
Assignees
Milestone

Comments

@rsc
Copy link
Contributor

@rsc rsc commented Feb 24, 2015

If you parse

func f() {
    goto label
label:
}

Then you get a LabeledStmt containing an EmptyStmt, and the EmptyStmt.Semicolon has as its value the offset of the } character, and then EmptyStmt.Semicolon.End() returns that+1, which means that the End is recorded as just beyond the closing brace. If you try to delete the statement from the program using the reported boundaries, you end up deleting the brace.

Full test case:

package main

import (
    "fmt"
    "go/ast"
    "go/parser"
    "go/token"
    "log"
)

func main() {
    fset := token.NewFileSet()
    f, err := parser.ParseFile(fset, "x.go", prog, 0)
    if err != nil {
        log.Fatal(err)
    }
    ast.Inspect(f, func(x ast.Node) bool {
        if x != nil {
            fmt.Printf("%T at %d,%d\n", x, x.Pos(), x.End())
        }
        return true
    })
}

var prog = `package p

func f() {
    goto L
L:
}
`

Prints:

*ast.File at 1,35
*ast.Ident at 9,10
*ast.FuncDecl at 12,35
*ast.Ident at 17,18
*ast.FuncType at 12,20
*ast.FieldList at 18,20
*ast.BlockStmt at 21,35
*ast.BranchStmt at 24,30
*ast.Ident at 29,30
*ast.LabeledStmt at 31,35
*ast.Ident at 31,32
*ast.EmptyStmt at 34,35

You can see that the EmptyStmt has the same end as the BlockStmt (and for that matter the FuncDecl and File).
It should be nested inside those.

I am kind of confused by the definition of EmptyStmt.Semicolon. What is the "preceding semicolon" when there's no semicolon due to semicolon insertion? It's tempting to say it is the \n, but there's not always a \n either.

Just to explore, consider changing var prog to

var prog = `package p;func f(){goto L;L:}`

Then the +1 in Semicolon.End cannot be right, because there is nothing safe to +1 over. Contrast this with:

var prog = `package p;func f(){goto L;L:;}`

It seems like the fix requires an extra field in EmptyStmt to distinguish real semicolons from implicit semicolons.

@rsc rsc added this to the Go1.5 milestone Feb 24, 2015
@griesemer griesemer closed this Feb 24, 2015
@griesemer griesemer reopened this Feb 24, 2015
@griesemer griesemer closed this in 5ce9fde Feb 24, 2015
@golang golang locked and limited conversation to collaborators Jun 25, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.