Commit 2a5cf48f authored by Robert Griesemer's avatar Robert Griesemer

cmd/compile: print columns (not just lines) in error messages

Compiler errors now show the exact line and line byte offset (sometimes
called "column") of where an error occured. For `go tool compile x.go`:

	package p
	const c int = false
	//line foo.go:123
	type t intg

reports

	x.go:2:7: cannot convert false to type int
	foo.go:123[x.go:4:8]: undefined: intg

(Some errors use the "wrong" position for the error message; arguably
the byte offset for the first error should be 15, the position of 'false',
rathen than 7, the position of 'c'. But that is an indepedent issue.)

The byte offset (column) values are measured in bytes; they start at 1,
matching the convention used by editors and IDEs.

Positions modified by //line directives show the line offset only for the
actual source location (in square brackets), not for the "virtual" file and
line number because that code is likely generated and the //line directive
only provides line information.

Because the new format might break existing tools or scripts, printing
of line offsets can be disabled with the new compiler flag -C. We plan
to remove this flag eventually.

Fixes #10324.

Change-Id: I493f5ee6e78457cf7b00025aba6b6e28e50bb740
Reviewed-on: https://go-review.googlesource.com/37970Reviewed-by: default avatarMatthew Dempsky <mdempsky@google.com>
parent 1f9f0ea3
...@@ -17,7 +17,7 @@ check() { ...@@ -17,7 +17,7 @@ check() {
expect() { expect() {
file=$1 file=$1
shift shift
if go build $file >errs 2>&1; then if go build -gcflags=-C $file >errs 2>&1; then
echo 1>&2 misc/cgo/errors/test.bash: BUG: expected cgo to fail on $file but it succeeded echo 1>&2 misc/cgo/errors/test.bash: BUG: expected cgo to fail on $file but it succeeded
exit 1 exit 1
fi fi
......
...@@ -166,6 +166,7 @@ func Main() { ...@@ -166,6 +166,7 @@ func Main() {
flag.BoolVar(&compiling_runtime, "+", false, "compiling runtime") flag.BoolVar(&compiling_runtime, "+", false, "compiling runtime")
obj.Flagcount("%", "debug non-static initializers", &Debug['%']) obj.Flagcount("%", "debug non-static initializers", &Debug['%'])
obj.Flagcount("B", "disable bounds checking", &Debug['B']) obj.Flagcount("B", "disable bounds checking", &Debug['B'])
obj.Flagcount("C", "disable printing of columns in error messages", &Debug['C']) // TODO(gri) remove eventually
flag.StringVar(&localimport, "D", "", "set relative `path` for local imports") flag.StringVar(&localimport, "D", "", "set relative `path` for local imports")
obj.Flagcount("E", "debug symbol export", &Debug['E']) obj.Flagcount("E", "debug symbol export", &Debug['E'])
obj.Flagfn1("I", "add `directory` to import search path", addidir) obj.Flagfn1("I", "add `directory` to import search path", addidir)
......
...@@ -87,7 +87,7 @@ func hcrash() { ...@@ -87,7 +87,7 @@ func hcrash() {
} }
func linestr(pos src.XPos) string { func linestr(pos src.XPos) string {
return Ctxt.OutermostPos(pos).String() return Ctxt.OutermostPos(pos).Format(Debug['C'] == 0)
} }
// lasterror keeps track of the most recently issued error. // lasterror keeps track of the most recently issued error.
......
...@@ -59,7 +59,7 @@ func Getgoextlinkenabled() string { ...@@ -59,7 +59,7 @@ func Getgoextlinkenabled() string {
} }
func (p *Prog) Line() string { func (p *Prog) Line() string {
return p.Ctxt.OutermostPos(p.Pos).String() return p.Ctxt.OutermostPos(p.Pos).Format(false)
} }
var armCondCode = []string{ var armCondCode = []string{
......
...@@ -13,8 +13,7 @@ import "strconv" ...@@ -13,8 +13,7 @@ import "strconv"
// position base and zero line number). // position base and zero line number).
// //
// The (line, column) values refer to a position in a file independent of any // The (line, column) values refer to a position in a file independent of any
// position base ("absolute" position). Line numbers start at 1, column values // position base ("absolute" file position).
// start at 0 and are byte offsets from the beginning of the line.
// //
// The position base is used to determine the "relative" position, that is the // The position base is used to determine the "relative" position, that is the
// filename and line number relative to the position base. If the base refers // filename and line number relative to the position base. If the base refers
...@@ -80,30 +79,42 @@ func (p Pos) AbsFilename() string { return p.base.AbsFilename() } ...@@ -80,30 +79,42 @@ func (p Pos) AbsFilename() string { return p.base.AbsFilename() }
func (p Pos) SymFilename() string { return p.base.SymFilename() } func (p Pos) SymFilename() string { return p.base.SymFilename() }
func (p Pos) String() string { func (p Pos) String() string {
return p.Format(true)
}
// Format formats a position as "filename:line" or "filename:line:column",
// controlled by the showCol flag.
// If the position is relative to a line directive, the original position
// is appended in square brackets without column (since the column doesn't
// change).
func (p Pos) Format(showCol bool) string {
if !p.IsKnown() { if !p.IsKnown() {
return "<unknown line number>" return "<unknown line number>"
} }
s := posString(p.Filename(), p.Line(), p.Col())
if b := p.base; b == b.Pos().base { if b := p.base; b == b.Pos().base {
// base is file base (incl. nil) // base is file base (incl. nil)
return s return format(p.Filename(), p.Line(), p.Col(), showCol)
} }
// base is relative // base is relative
return posString(p.RelFilename(), p.RelLine(), p.Col()) + "[" + s + "]" // Print the column only for the original position since the
// relative position's column information may be bogus (it's
// typically generated code and we can't say much about the
// original source at that point but for the file:line info
// that's provided via a line directive).
// TODO(gri) This may not be true if we have an inlining base.
// We may want to differentiate at some point.
return format(p.RelFilename(), p.RelLine(), 0, false) +
"[" + format(p.Filename(), p.Line(), p.Col(), showCol) + "]"
} }
// Don't print column numbers because existing tests may not work anymore. // format formats a (filename, line, col) tuple as "filename:line" (showCol
// It's a variable for now so that the tests can enable it. // is false) or "filename:line:column" (showCol is true).
// TODO(gri) fix this func format(filename string, line, col uint, showCol bool) string {
var printColumn = false
// posString formats a (filename, line, col) tuple as a printable position.
func posString(filename string, line, col uint) string {
s := filename + ":" + strconv.FormatUint(uint64(line), 10) s := filename + ":" + strconv.FormatUint(uint64(line), 10)
// col == colMax is interpreted as unknown column value // col == colMax is interpreted as unknown column value
if printColumn && col < colMax { if showCol && col < colMax {
s += ":" + strconv.FormatUint(uint64(col), 10) s += ":" + strconv.FormatUint(uint64(col), 10)
} }
return s return s
......
...@@ -10,8 +10,6 @@ import ( ...@@ -10,8 +10,6 @@ import (
) )
func TestPos(t *testing.T) { func TestPos(t *testing.T) {
printColumn = true
f0 := NewFileBase("", "") f0 := NewFileBase("", "")
f1 := NewFileBase("f1", "f1") f1 := NewFileBase("f1", "f1")
f2 := NewLinePragmaBase(Pos{}, "f2", 10) f2 := NewLinePragmaBase(Pos{}, "f2", 10)
...@@ -41,15 +39,15 @@ func TestPos(t *testing.T) { ...@@ -41,15 +39,15 @@ func TestPos(t *testing.T) {
{MakePos(nil, 2, 3), ":2:3", "", 2, 3, "", 2}, {MakePos(nil, 2, 3), ":2:3", "", 2, 3, "", 2},
{MakePos(f0, 2, 3), ":2:3", "", 2, 3, "", 2}, {MakePos(f0, 2, 3), ":2:3", "", 2, 3, "", 2},
{MakePos(f1, 1, 1), "f1:1:1", "f1", 1, 1, "f1", 1}, {MakePos(f1, 1, 1), "f1:1:1", "f1", 1, 1, "f1", 1},
{MakePos(f2, 7, 10), "f2:16:10[:7:10]", "", 7, 10, "f2", 16}, {MakePos(f2, 7, 10), "f2:16[:7:10]", "", 7, 10, "f2", 16},
{MakePos(f3, 12, 7), "f3:101:7[f1:12:7]", "f1", 12, 7, "f3", 101}, {MakePos(f3, 12, 7), "f3:101[f1:12:7]", "f1", 12, 7, "f3", 101},
{MakePos(f4, 25, 1), "f4:114:1[f3:25:1]", "f3", 25, 1, "f4", 114}, {MakePos(f4, 25, 1), "f4:114[f3:25:1]", "f3", 25, 1, "f4", 114},
// positions from issue #19392 // positions from issue #19392
{MakePos(fc, 4, 0), "c.go:10:0[p.go:4:0]", "p.go", 4, 0, "c.go", 10}, {MakePos(fc, 4, 0), "c.go:10[p.go:4:0]", "p.go", 4, 0, "c.go", 10},
{MakePos(ft, 7, 0), "t.go:20:0[p.go:7:0]", "p.go", 7, 0, "t.go", 20}, {MakePos(ft, 7, 0), "t.go:20[p.go:7:0]", "p.go", 7, 0, "t.go", 20},
{MakePos(fv, 10, 0), "v.go:30:0[p.go:10:0]", "p.go", 10, 0, "v.go", 30}, {MakePos(fv, 10, 0), "v.go:30[p.go:10:0]", "p.go", 10, 0, "v.go", 30},
{MakePos(ff, 13, 0), "f.go:40:0[p.go:13:0]", "p.go", 13, 0, "f.go", 40}, {MakePos(ff, 13, 0), "f.go:40[p.go:13:0]", "p.go", 13, 0, "f.go", 40},
} { } {
pos := test.pos pos := test.pos
if got := pos.String(); got != test.string { if got := pos.String(); got != test.string {
...@@ -120,8 +118,6 @@ func TestPredicates(t *testing.T) { ...@@ -120,8 +118,6 @@ func TestPredicates(t *testing.T) {
} }
func TestLico(t *testing.T) { func TestLico(t *testing.T) {
printColumn = true
for _, test := range []struct { for _, test := range []struct {
x lico x lico
string string string string
...@@ -140,7 +136,7 @@ func TestLico(t *testing.T) { ...@@ -140,7 +136,7 @@ func TestLico(t *testing.T) {
{makeLico(lineMax+1, colMax+1), fmt.Sprintf(":%d", lineMax), lineMax, 0}, {makeLico(lineMax+1, colMax+1), fmt.Sprintf(":%d", lineMax), lineMax, 0},
} { } {
x := test.x x := test.x
if got := posString("", x.Line(), x.Col()); got != test.string { if got := format("", x.Line(), x.Col(), true); got != test.string {
t.Errorf("%s: got %q", test.string, got) t.Errorf("%s: got %q", test.string, got)
} }
} }
......
...@@ -585,7 +585,8 @@ func (t *test) run() { ...@@ -585,7 +585,8 @@ func (t *test) run() {
t.err = fmt.Errorf("unimplemented action %q", action) t.err = fmt.Errorf("unimplemented action %q", action)
case "errorcheck": case "errorcheck":
cmdline := []string{"go", "tool", "compile", "-e", "-o", "a.o"} // TODO(gri) remove need for -C (disable printing of columns in error messages)
cmdline := []string{"go", "tool", "compile", "-C", "-e", "-o", "a.o"}
// No need to add -dynlink even if linkshared if we're just checking for errors... // No need to add -dynlink even if linkshared if we're just checking for errors...
cmdline = append(cmdline, flags...) cmdline = append(cmdline, flags...)
cmdline = append(cmdline, long) cmdline = append(cmdline, long)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment