column numbers and -apply don't always work with cgo
mewmew opened this issue · 5 comments
$ go get azul3d.org/engine/gfx/internal/gl/2.0/gl
$ unconvert azul3d.org/engine/gfx/internal/gl/2.0/gl
panic: illegal file offset
goroutine 1 [running]:
panic(0x603c00, 0xc824298740)
/home/u/go/src/runtime/panic.go:500 +0x189
go/token.(*File).Pos(0xc824684420, 0xd54, 0x1)
/home/u/go/src/go/token/position.go:237 +0x88
main.print(0xc8201d7ef0, 0x43, 0xc8247ec0d0)
/home/u/goget/src/github.com/mdempsky/unconvert/unconvert.go:119 +0x23a
main.main()
/home/u/goget/src/github.com/mdempsky/unconvert/unconvert.go:210 +0x38f
Quoting from golang.org/x/tools/go/loader:
// The advantage of this approach is its fidelity to 'go build'. The
// downside is that the token.Position.Offset for each AST node is
// incorrect, being an offset within the temporary file. Line numbers
// should still be correct because of the //line comments.
Of course, I'm relying on token.Position.Offset
to identify AST nodes within a file. Doh.
I guess I need to switch to using (Line
, Column
) instead.
See @tamird's tests at #12 (comment)
TL;DR: We really only have accurate line number info for cgo-processed files, not even column number.
To implement it correctly, we'll need some sort of matching heuristic to correlate intraline position info between the cgo-using files before and after cgo processing.
I think this could be fixed by having upstream use /*line*/
comments so that we have accurate column numbers after cgo mutations.
Looks like we'll have that in Go 1.12 actually: golang/go#26745.
Confirmed that this will be fixed for Go 1.12, including support for -apply.