cmd/compile/internal/syntax: fix error handling for Read/Parse calls

- define syntax.Error for cleaner error reporting
- abort parsing after first error if no error handler is installed
- make sure to always report the first error, if any
- document behavior of API calls
- while at it: rename ReadXXX -> ParseXXX (clearer)
- adjust cmd/compile noder.go accordingly

Fixes #17774.

Change-Id: I7893eedea454a64acd753e32f7a8bf811ddbb03c
Reviewed-on: https://go-review.googlesource.com/32950
Reviewed-by: Matthew Dempsky <[email protected]>

Author: griesemer

src/cmd/compile/internal/gc/noder.go

@@ -6,6 +6,7 @@ package gc
 
 import (
 	"fmt"
+	"os"
 	"strconv"
 	"strings"
 	"unicode/utf8"
@@ -14,18 +15,21 @@ import (
 )
 
 func parseFile(filename string) {
-	p := noder{baseline: lexlineno}
-	file, err := syntax.ReadFile(filename, p.error, p.pragma, 0)
+	src, err := os.Open(filename)
 	if err != nil {
-		fmt.Printf("parse %s: %v\n", filename, err)
+		fmt.Println(err)
 		errorexit()
 	}
+	defer src.Close()
+
+	p := noder{baseline: lexlineno}
+	file, _ := syntax.Parse(src, p.error, p.pragma, 0) // errors are tracked via p.error
 
 	p.file(file)
 
 	if !imported_unsafe {
 		for _, x := range p.linknames {
-			p.error(0, x, "//go:linkname only allowed in Go files that import \"unsafe\"")
+			p.error(syntax.Error{0, x, "//go:linkname only allowed in Go files that import \"unsafe\""})
 		}
 	}
 
@@ -1003,8 +1007,16 @@ func (p *noder) lineno(n syntax.Node) {
 	lineno = p.baseline + l - 1
 }
 
-func (p *noder) error(_, line int, msg string) {
-	yyerrorl(p.baseline+int32(line)-1, "%s", msg)
+func (p *noder) error(err error) {
+	line := p.baseline
+	var msg string
+	if err, ok := err.(syntax.Error); ok {
+		line += int32(err.Line) - 1
+		msg = err.Msg
+	} else {
+		msg = err.Error()
+	}
+	yyerrorl(line, "%s", msg)
 }
 
 func (p *noder) pragma(pos, line int, text string) syntax.Pragma {
@@ -1020,7 +1032,7 @@ func (p *noder) pragma(pos, line int, text string) syntax.Pragma {
 			break
 		}
 		if n > 1e8 {
-			p.error(pos, line, "line number out of range")
+			p.error(syntax.Error{pos, line, "line number out of range"})
 			errorexit()
 		}
 		if n <= 0 {
@@ -1036,7 +1048,7 @@ func (p *noder) pragma(pos, line int, text string) syntax.Pragma {
 
 		f := strings.Fields(text)
 		if len(f) != 3 {
-			p.error(pos, line, "usage: //go:linkname localname linkname")
+			p.error(syntax.Error{pos, line, "usage: //go:linkname localname linkname"})
 			break
 		}
 		lookup(f[1]).Linkname = f[2]

src/cmd/compile/internal/syntax/dumper_test.go

@@ -14,7 +14,7 @@ func TestDump(t *testing.T) {
 		t.Skip("skipping test in short mode")
 	}
 
-	ast, err := ReadFile(*src, nil, nil, 0)
+	ast, err := ParseFile(*src, nil, nil, 0)
 	if err != nil {
 		t.Fatal(err)
 	}

src/cmd/compile/internal/syntax/parser.go

@@ -24,27 +24,16 @@ type parser struct {
 	fnest  int    // function nesting level (for error handling)
 	xnest  int    // expression nesting level (for complit ambiguity resolution)
 	indent []byte // tracing support
-
-	nerrors int // error count
 }
 
 type parserError string // for error recovery if no error handler was installed
 
 func (p *parser) init(src io.Reader, errh ErrorHandler, pragh PragmaHandler) {
-	p.scanner.init(src, func(pos, line int, msg string) {
-		p.nerrors++
-		if !debug && errh != nil {
-			errh(pos, line, msg)
-			return
-		}
-		panic(parserError(fmt.Sprintf("%d: %s\n", line, msg)))
-	}, pragh)
+	p.scanner.init(src, errh, pragh)
 
 	p.fnest = 0
 	p.xnest = 0
 	p.indent = nil
-
-	p.nerrors = 0
 }
 
 func (p *parser) got(tok token) bool {
@@ -76,7 +65,7 @@ func (p *parser) syntax_error_at(pos, line int, msg string) {
 		defer p.trace("syntax_error (" + msg + ")")()
 	}
 
-	if p.tok == _EOF && p.nerrors > 0 {
+	if p.tok == _EOF && p.first != nil {
 		return // avoid meaningless follow-up errors
 	}
 
@@ -207,7 +196,7 @@ func (p *parser) file() *File {
 	p.want(_Semi)
 
 	// don't bother continuing if package clause has errors
-	if p.nerrors > 0 {
+	if p.first != nil {
 		return nil
 	}
 

src/cmd/compile/internal/syntax/parser_test.go

@@ -22,7 +22,7 @@ var src = flag.String("src", "parser.go", "source file to parse")
 var verify = flag.Bool("verify", false, "verify idempotent printing")
 
 func TestParse(t *testing.T) {
-	_, err := ReadFile(*src, nil, nil, 0)
+	_, err := ParseFile(*src, nil, nil, 0)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -52,7 +52,7 @@ func TestStdLib(t *testing.T) {
 				if debug {
 					fmt.Printf("parsing %s\n", filename)
 				}
-				ast, err := ReadFile(filename, nil, nil, 0)
+				ast, err := ParseFile(filename, nil, nil, 0)
 				if err != nil {
 					t.Error(err)
 					return
@@ -133,7 +133,7 @@ func verifyPrint(filename string, ast1 *File) {
 		panic(err)
 	}
 
-	ast2, err := ReadBytes(buf1.Bytes(), nil, nil, 0)
+	ast2, err := ParseBytes(buf1.Bytes(), nil, nil, 0)
 	if err != nil {
 		panic(err)
 	}
@@ -157,8 +157,28 @@ func verifyPrint(filename string, ast1 *File) {
 }
 
 func TestIssue17697(t *testing.T) {
-	_, err := ReadBytes(nil, nil, nil, 0) // return with parser error, don't panic
+	_, err := ParseBytes(nil, nil, nil, 0) // return with parser error, don't panic
 	if err == nil {
 		t.Errorf("no error reported")
 	}
 }
+
+func TestParseFile(t *testing.T) {
+	_, err := ParseFile("", nil, nil, 0)
+	if err == nil {
+		t.Error("missing io error")
+	}
+
+	var first error
+	_, err = ParseFile("", func(err error) {
+		if first == nil {
+			first = err
+		}
+	}, nil, 0)
+	if err == nil || first == nil {
+		t.Error("missing io error")
+	}
+	if err != first {
+		t.Error("got %v; want first error %v", err, first)
+	}
+}

src/cmd/compile/internal/syntax/printer_test.go

@@ -15,7 +15,7 @@ func TestPrint(t *testing.T) {
 		t.Skip("skipping test in short mode")
 	}
 
-	ast, err := ReadFile(*src, nil, nil, 0)
+	ast, err := ParseFile(*src, nil, nil, 0)
 	if err != nil {
 		t.Fatal(err)
 	}

src/cmd/compile/internal/syntax/scanner_test.go

@@ -322,21 +322,22 @@ func TestScanErrors(t *testing.T) {
 	} {
 		var s scanner
 		nerrors := 0
-		s.init(&bytesReader{[]byte(test.src)}, func(pos, line int, msg string) {
+		s.init(&bytesReader{[]byte(test.src)}, func(err error) {
 			nerrors++
 			// only check the first error
+			e := err.(Error) // we know it's an Error
 			if nerrors == 1 {
-				if msg != test.msg {
-					t.Errorf("%q: got msg = %q; want %q", test.src, msg, test.msg)
+				if e.Msg != test.msg {
+					t.Errorf("%q: got msg = %q; want %q", test.src, e.Msg, test.msg)
 				}
-				if pos != test.pos {
-					t.Errorf("%q: got pos = %d; want %d", test.src, pos, test.pos)
+				if e.Pos != test.pos {
+					t.Errorf("%q: got pos = %d; want %d", test.src, e.Pos, test.pos)
 				}
-				if line != test.line {
-					t.Errorf("%q: got line = %d; want %d", test.src, line, test.line)
+				if e.Line != test.line {
+					t.Errorf("%q: got line = %d; want %d", test.src, e.Line, test.line)
 				}
 			} else if nerrors > 1 {
-				t.Errorf("%q: got unexpected %q at pos = %d, line = %d", test.src, msg, pos, line)
+				t.Errorf("%q: got unexpected %q at pos = %d, line = %d", test.src, e.Msg, e.Pos, e.Line)
 			}
 		}, nil)
 

src/cmd/compile/internal/syntax/source.go

@@ -5,7 +5,6 @@
 package syntax
 
 import (
-	"fmt"
 	"io"
 	"unicode/utf8"
 )
@@ -16,8 +15,9 @@ import (
 //        suf     r0  r            w
 
 type source struct {
-	src  io.Reader
-	errh ErrorHandler
+	src   io.Reader
+	errh  ErrorHandler
+	first error // first error encountered
 
 	// source buffer
 	buf         [4 << 10]byte
@@ -34,6 +34,7 @@ type source struct {
 func (s *source) init(src io.Reader, errh ErrorHandler) {
 	s.src = src
 	s.errh = errh
+	s.first = nil
 
 	s.buf[0] = utf8.RuneSelf // terminate with sentinel
 	s.offs = 0
@@ -50,11 +51,14 @@ func (s *source) error(msg string) {
 }
 
 func (s *source) error_at(pos, line int, msg string) {
-	if s.errh != nil {
-		s.errh(pos, line, msg)
-		return
+	err := Error{pos, line, msg}
+	if s.first == nil {
+		s.first = err
 	}
-	panic(fmt.Sprintf("%d: %s", line, msg))
+	if s.errh == nil {
+		panic(s.first)
+	}
+	s.errh(err)
 }
 
 // pos0 returns the byte position of the last character read.