]> Cypherpunks repositories - gostls13.git/commitdiff
cmd/compile: print columns (not just lines) in error messages
authorRobert Griesemer <gri@golang.org>
Wed, 8 Mar 2017 22:26:23 +0000 (14:26 -0800)
committerRobert Griesemer <gri@golang.org>
Thu, 9 Mar 2017 23:29:49 +0000 (23:29 +0000)
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/37970
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
misc/cgo/errors/test.bash
src/cmd/compile/internal/gc/main.go
src/cmd/compile/internal/gc/subr.go
src/cmd/internal/obj/util.go
src/cmd/internal/src/pos.go
src/cmd/internal/src/pos_test.go
test/run.go

index 05261e9d767c1eb7d34165f869ee9d70b84e8c1f..27d7dc1cfeef1120969e715d6732891a35c61641 100755 (executable)
@@ -17,7 +17,7 @@ check() {
 expect() {
        file=$1
        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
                exit 1
        fi
index 2c4615cba11eeccb6065735b61968de5a8be99cf..49227ecaf66175f4cda02bf1e15c6556c198ad10 100644 (file)
@@ -166,6 +166,7 @@ func Main() {
        flag.BoolVar(&compiling_runtime, "+", false, "compiling runtime")
        obj.Flagcount("%", "debug non-static initializers", &Debug['%'])
        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")
        obj.Flagcount("E", "debug symbol export", &Debug['E'])
        obj.Flagfn1("I", "add `directory` to import search path", addidir)
index 923205ca39b1e50f99271d70d6a6671fd6abe3b1..880f1350d334318b20f6dcf5d3f599cebd3ff4c7 100644 (file)
@@ -87,7 +87,7 @@ func hcrash() {
 }
 
 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.
index f1f832362b922ad413051c527426954f991cf755..69898c73851933b118a65a43d01ba72f3f4d8519 100644 (file)
@@ -59,7 +59,7 @@ func Getgoextlinkenabled() string {
 }
 
 func (p *Prog) Line() string {
-       return p.Ctxt.OutermostPos(p.Pos).String()
+       return p.Ctxt.OutermostPos(p.Pos).Format(false)
 }
 
 var armCondCode = []string{
index 04e2068d7c3ab2d4e6d0ebad930e38bc9e7c5883..a1ea3fcdac71acfe35b107872a709fda0b3cfd87 100644 (file)
@@ -13,8 +13,7 @@ import "strconv"
 // position base and zero line number).
 //
 // 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
-// start at 0 and are byte offsets from the beginning of the line.
+// position base ("absolute" file position).
 //
 // 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
@@ -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) 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() {
                return "<unknown line number>"
        }
 
-       s := posString(p.Filename(), p.Line(), p.Col())
        if b := p.base; b == b.Pos().base {
                // base is file base (incl. nil)
-               return s
+               return format(p.Filename(), p.Line(), p.Col(), showCol)
        }
 
        // 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.
-// It's a variable for now so that the tests can enable it.
-// TODO(gri) fix this
-var printColumn = false
-
-// posString formats a (filename, line, col) tuple as a printable position.
-func posString(filename string, line, col uint) string {
+// format formats a (filename, line, col) tuple as "filename:line" (showCol
+// is false) or "filename:line:column" (showCol is true).
+func format(filename string, line, col uint, showCol bool) string {
        s := filename + ":" + strconv.FormatUint(uint64(line), 10)
        // col == colMax is interpreted as unknown column value
-       if printColumn && col < colMax {
+       if showCol && col < colMax {
                s += ":" + strconv.FormatUint(uint64(col), 10)
        }
        return s
index 3dc5d37b15fbedb53faa1f55dff11af3701454c7..a101bc10b129e60c09f19d5d7e346e10eef0c7bb 100644 (file)
@@ -10,8 +10,6 @@ import (
 )
 
 func TestPos(t *testing.T) {
-       printColumn = true
-
        f0 := NewFileBase("", "")
        f1 := NewFileBase("f1", "f1")
        f2 := NewLinePragmaBase(Pos{}, "f2", 10)
@@ -41,15 +39,15 @@ func TestPos(t *testing.T) {
                {MakePos(nil, 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(f2, 7, 10), "f2:16:10[:7:10]", "", 7, 10, "f2", 16},
-               {MakePos(f3, 12, 7), "f3:101:7[f1:12:7]", "f1", 12, 7, "f3", 101},
-               {MakePos(f4, 25, 1), "f4:114:1[f3:25:1]", "f3", 25, 1, "f4", 114},
+               {MakePos(f2, 7, 10), "f2:16[:7:10]", "", 7, 10, "f2", 16},
+               {MakePos(f3, 12, 7), "f3:101[f1:12:7]", "f1", 12, 7, "f3", 101},
+               {MakePos(f4, 25, 1), "f4:114[f3:25:1]", "f3", 25, 1, "f4", 114},
 
                // positions from issue #19392
-               {MakePos(fc, 4, 0), "c.go:10:0[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(fv, 10, 0), "v.go:30:0[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(fc, 4, 0), "c.go:10[p.go:4:0]", "p.go", 4, 0, "c.go", 10},
+               {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[p.go:10:0]", "p.go", 10, 0, "v.go", 30},
+               {MakePos(ff, 13, 0), "f.go:40[p.go:13:0]", "p.go", 13, 0, "f.go", 40},
        } {
                pos := test.pos
                if got := pos.String(); got != test.string {
@@ -120,8 +118,6 @@ func TestPredicates(t *testing.T) {
 }
 
 func TestLico(t *testing.T) {
-       printColumn = true
-
        for _, test := range []struct {
                x         lico
                string    string
@@ -140,7 +136,7 @@ func TestLico(t *testing.T) {
                {makeLico(lineMax+1, colMax+1), fmt.Sprintf(":%d", lineMax), lineMax, 0},
        } {
                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)
                }
        }
index 19ca3287651388d34ef71171930e708b6dd9a4af..3a97cf2c3be361d63b894ad44d2dfc15f815c79c 100644 (file)
@@ -585,7 +585,8 @@ func (t *test) run() {
                t.err = fmt.Errorf("unimplemented action %q", action)
 
        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...
                cmdline = append(cmdline, flags...)
                cmdline = append(cmdline, long)