fix error reporting in recursive definitions

char.go

@@ -97,7 +97,6 @@ func (p *charParser) parse(c *context) {
 	if tok, ok := c.token(); !ok || !p.match(tok) {
 		if c.offset > c.failOffset {
 			c.failOffset = c.offset
-			// println("clearing failing parser")
 			c.failingParser = nil
 		}
 

choice.go

@@ -163,8 +163,6 @@ func (p *choiceParser) parse(c *context) {
 		return
 	}
 
-	// println("parsing choice", p.name, c.offset)
-
 	c.results.markPending(c.offset, p.id)
 	from := c.offset
 	to := c.offset
@@ -173,10 +171,10 @@ func (p *choiceParser) parse(c *context) {
 	var optionIndex int
 	var foundMatch bool
 
-	// TODO:
-	// - if there is a failure already, it should be left alone
-	// - what if reading more means that the previous failurs don't count
 	initialFailOffset := c.failOffset
+	initialFailingParser := c.failingParser
+	failOffset := initialFailOffset
+	var failingParser parser
 
 	for {
 		foundMatch = false
@@ -186,6 +184,13 @@ func (p *choiceParser) parse(c *context) {
 			p.options[optionIndex].parse(c)
 			optionIndex++
 
+			if !c.matchLast {
+				if c.failOffset > failOffset {
+					failOffset = c.failOffset
+					failingParser = c.failingParser
+				}
+			}
+
 			if !c.matchLast || match && c.offset <= to {
 				c.offset = from
 				continue
@@ -204,9 +209,12 @@ func (p *choiceParser) parse(c *context) {
 	}
 
 	if match {
-		if to >= c.failOffset {
+		if to > initialFailOffset {
 			c.failOffset = -1
 			c.failingParser = nil
+		} else {
+			c.failOffset = initialFailOffset
+			c.failingParser = initialFailingParser
 		}
 
 		c.success(to)
@@ -214,11 +222,12 @@ func (p *choiceParser) parse(c *context) {
 		return
 	}
 
-	// TODO:
+	if failOffset > initialFailOffset {
-	// - what if all of it pending?
+		c.failOffset = failOffset
-	if c.failOffset > initialFailOffset && c.failingParser == nil {
+		c.failingParser = failingParser
-		if p.commitType()&userDefined != 0 && p.commitType()&Whitespace == 0 {
+		if c.failingParser == nil &&
-			// println("recording choice failure", p.name, from, c.failOffset)
+			p.commitType()&userDefined != 0 &&
+			p.commitType()&Whitespace == 0 {
 			c.failingParser = p
 		}
 	}

context.go

@@ -109,7 +109,6 @@ func (c *context) recordFailure(offset int, p parser) {
 
 	c.failOffset = offset
 	if p.commitType()&userDefined != 0 && p.commitType()&Whitespace == 0 {
-		// println("setting failing sequence parser", p.nodeName(), offset)
 		c.failingParser = p
 	}
 }
@@ -146,20 +145,15 @@ func (c *context) parseError(p parser) error {
 func (c *context) finalizeParse(root parser) error {
 	p := c.failingParser
 	if p == nil {
-		// println("failing parser is nil")
 		p = root
 	}
 
-	// println("failing parser is", p.nodeName())
-
 	to, match, found := c.results.longestResult(0, root.nodeID())
 
 	if found && match && to < c.readOffset {
-		// println("forcing root", found, match, to, c.readOffset)
 		return c.parseError(p)
 	}
 
-	// TODO: test both cases
 	if !found || !match {
 		return c.parseError(p)
 	}

errors_test.go

@@ -2,105 +2,135 @@ package treerack
 
 import (
 	"bytes"
+	"reflect"
 	"testing"
 )
 
+func checkParseError(left, right ParseError) bool {
+	left.registry = nil
+	right.registry = nil
+	return reflect.DeepEqual(left, right)
+}
+
 func TestError(t *testing.T) {
 	type testItem struct {
-		title      string
+		title  string
-		syntax     string
+		syntax string
-		text       string
+		text   string
-		offset     int
+		perr   ParseError
-		line       int
-		column     int
-		definition string
 	}
 
 	for _, test := range []testItem{{
-		title:      "single def, empty text",
+		title:  "single def, empty text",
-		syntax:     `a = "a"`,
+		syntax: `a = "a"`,
-		definition: "a",
+		perr: ParseError{
+			Definition: "a",
+		},
 	}, {
-		title:      "single def, wrong text",
+		title:  "single def, wrong text",
-		syntax:     `a = "a"`,
+		syntax: `a = "a"`,
-		text:       "b",
+		text:   "b",
-		definition: "a",
+		perr: ParseError{
+			Definition: "a",
+		},
 	}, {
-		title:      "single optional def, wrong text",
+		title:  "single optional def, wrong text",
-		syntax:     `a = "a"?`,
+		syntax: `a = "a"?`,
-		text:       "b",
+		text:   "b",
-		definition: "a",
+		perr: ParseError{
+			Definition: "a",
+		},
 	}, {
-		title:      "error on second line, second column",
+		title:  "error on second line, second column",
-		syntax:     `a = [a\n]*`,
+		syntax: `a = [a\n]*`,
-		text:       "aa\nabaa\naa",
+		text:   "aa\nabaa\naa",
-		offset:     4,
+		perr: ParseError{
-		line:       1,
+			Offset:     4,
-		column:     1,
+			Line:       1,
-		definition: "a",
+			Column:     1,
+			Definition: "a",
+		},
 	}, {
-		title:      "multiple definitions",
+		title:  "multiple definitions",
-		syntax:     `a = "aa"; A:root = a`,
+		syntax: `a = "aa"; A:root = a`,
-		text:       "ab",
+		text:   "ab",
-		offset:     1,
+		perr: ParseError{
-		column:     1,
+			Offset:     1,
-		definition: "a",
+			Column:     1,
+			Definition: "a",
+		},
 	}, {
-		title:      "choice, options succeed",
+		title:  "choice, options succeed",
-		syntax:     `a = "12"; b = "1"; c:root = a | b`,
+		syntax: `a = "12"; b = "1"; c:root = a | b`,
-		text:       "123",
+		text:   "123",
-		offset:     2,
+		perr: ParseError{
-		column:     2,
+			Offset:     2,
-		definition: "c",
+			Column:     2,
+			Definition: "c",
+		},
 	}, {
-		title:      "choice succeeds, document fails",
+		title:  "choice succeeds, document fails",
-		syntax:     `a = "12"; b = "1"; c:root = a | b`,
+		syntax: `a = "12"; b = "1"; c:root = a | b`,
-		text:       "13",
+		text:   "13",
-		offset:     1,
+		perr: ParseError{
-		column:     1,
+			Offset:     1,
-		definition: "c",
+			Column:     1,
+			Definition: "c",
+		},
 	}, {
-		title:      "choice fails",
+		title:  "choice fails",
-		syntax:     `a = "12"; b = "2"; c:root = a | b`,
+		syntax: `a = "12"; b = "2"; c:root = a | b`,
-		text:       "13",
+		text:   "13",
-		offset:     1,
+		perr: ParseError{
-		column:     1,
+			Offset:     1,
-		definition: "a",
+			Column:     1,
+			Definition: "a",
+		},
 	}, {
-		title:      "choice fails, longer option reported",
+		title:  "choice fails, longer option reported",
-		syntax:     `a = "12"; b = "134"; c:root = a | b`,
+		syntax: `a = "12"; b = "134"; c:root = a | b`,
-		text:       "135",
+		text:   "135",
-		offset:     2,
+		perr: ParseError{
-		column:     2,
+			Offset:     2,
-		definition: "b",
+			Column:     2,
+			Definition: "b",
+		},
 	}, {
-		title:      "failing choice on the failing branch",
+		title:  "failing choice on the failing branch",
-		syntax:     `a = "123"; b:root = a | "13"`,
+		syntax: `a = "123"; b:root = a | "13"`,
-		text:       "124",
+		text:   "124",
-		offset:     2,
+		perr: ParseError{
-		column:     2,
+			Offset:     2,
-		definition: "a",
+			Column:     2,
+			Definition: "a",
+		},
 	}, {
-		title:      "failing choice on a shorter branch",
+		title:  "failing choice on a shorter branch",
-		syntax:     `a = "13"; b:root = "123" | a`,
+		syntax: `a = "13"; b:root = "123" | a`,
-		text:       "124",
+		text:   "124",
-		offset:     2,
+		perr: ParseError{
-		column:     2,
+			Offset:     2,
-		definition: "b",
+			Column:     2,
+			Definition: "b",
+		},
 	}, {
-		title:      "longer failure on a later pass",
+		title:  "longer failure on a later pass",
-		syntax:     `a = "12"; b = "34"; c = "1" b; d:root = a | c`,
+		syntax: `a = "12"; b = "34"; c = "1" b; d:root = a | c`,
-		text:       "135",
+		text:   "135",
-		offset:     2,
+		perr: ParseError{
-		column:     2,
+			Offset:     2,
-		definition: "b",
+			Column:     2,
+			Definition: "b",
+		},
 	}, {
-		title:      "char as a choice option",
+		title:  "char as a choice option",
-		syntax:     `a = "12"; b = [a] | [b]; c = a b`,
+		syntax: `a = "12"; b = [a] | [b]; c = a b`,
-		text:       "12c",
+		text:   "12c",
-		offset:     2,
+		perr: ParseError{
-		column:     2,
+			Offset:     2,
-		definition: "b",
+			Column:     2,
+			Definition: "b",
+		},
 	}} {
 		t.Run(test.title, func(t *testing.T) {
 			s, err := openSyntaxString(test.syntax)
@@ -109,7 +139,6 @@ func TestError(t *testing.T) {
 				return
 			}
 
-			// println("starting")
 			_, err = s.Parse(bytes.NewBufferString(test.text))
 			if err == nil {
 				t.Error("failed to fail")
@@ -127,50 +156,105 @@ func TestError(t *testing.T) {
 				return
 			}
 
-			if perr.Offset != test.offset {
+			perr.Input = ""
-				t.Error("invalid error offset", perr.Offset, test.offset)
+			if !checkParseError(*perr, test.perr) {
-				return
+				t.Error("failed to return the right error")
-			}
-
-			if perr.Line != test.line {
-				t.Error("invalid line index", perr.Line, test.line)
-				return
-			}
-
-			if perr.Column != test.column {
-				t.Error("invalid column index", perr.Column, test.column)
-			}
-
-			if perr.Definition != test.definition {
-				t.Error("invalid definition", perr.Definition, test.definition)
 			}
 		})
 	}
 }
 
 func TestErrorRecursive(t *testing.T) {
-	// const doc = `a[b][1a]`
+	const syntax = `
-	const doc = `a[1a]`
+		ws:ws = " ";
+		symbol = [a-z]+;
+		function-application = expression "(" expression? ")";
+		expression = function-application | symbol;
+		doc:root = (expression (";" expression)*)+;
+	`
 
-	s, err := openSyntaxFile("examples/mml.treerack")
+	s, err := openSyntaxString(syntax)
 	if err != nil {
-		t.Error(err)
+		t.Fatal(err)
-		return
 	}
 
-	// println("\n<<<< starting >>>>\n")
+	for _, test := range []struct {
-	_, err = s.Parse(bytes.NewBufferString(doc))
+		title string
-	perr, ok := err.(*ParseError)
+		doc   string
-	if !ok {
+		perr  ParseError
-		t.Error("failed to return parse error")
+	}{{
-		return
+		title: "simple, open",
-	}
+		doc:   "a(",
+		perr: ParseError{
+			Offset:     1,
+			Column:     1,
+			Definition: "function-application",
+		},
+	}, {
+		title: "simple, close",
+		doc:   "a)",
+		perr: ParseError{
+			Offset:     1,
+			Column:     1,
+			Definition: "function-application",
+		},
+	}, {
+		title: "inner, open",
+		doc:   "a(b()",
+		perr: ParseError{
+			Offset:     1,
+			Column:     1,
+			Definition: "function-application",
+		},
+	}, {
+		title: "inner, close",
+		doc:   "a(b))",
+		perr: ParseError{
+			Offset:     4,
+			Column:     4,
+			Definition: "function-application",
+		},
+	}, {
+		title: "outer, open",
+		doc:   "a()b(",
+		perr: ParseError{
+			Offset:     4,
+			Column:     4,
+			Definition: "function-application",
+		},
+	}, {
+		title: "outer, close",
+		doc:   "a()b)",
+		perr: ParseError{
+			Offset:     4,
+			Column:     4,
+			Definition: "function-application",
+		},
+	}} {
+		t.Run(test.title, func(t *testing.T) {
+			_, err := s.Parse(bytes.NewBufferString(test.doc))
+			if err == nil {
+				t.Fatal("failed to fail")
+			}
 
-	t.Log(perr)
+			perr, ok := err.(*ParseError)
+			if !ok {
+				t.Fatal("invalid error type")
+			}
+
+			perr.Input = ""
+
+			if !checkParseError(*perr, test.perr) {
+				t.Error("failed to return the right error")
+				t.Log("got:     ", *perr)
+				t.Log("expected:", test.perr)
+			}
+		})
+	}
 }
 
 func TestErrorMessage(t *testing.T) {
-	const expected = "foo:4:10:failed to parse input, expecting: bar"
+	const expected = "foo:4:10:parse failed, parsing: bar"
 
 	perr := &ParseError{
 		Input:      "foo",

notes.txt

@@ -29,6 +29,9 @@ verify choice and sequence preference
 formatter
 pretty
 
+[optimization]
+try preallocate larger store chunks
+
 [problems]
 can the root be an alias? check the commit mechanism
 

sequence.go

@@ -307,17 +307,15 @@ func (p *sequenceParser) parse(c *context) {
 	from := c.offset
 	to := c.offset
 	var parsed bool
-	initialFailOffset := c.failOffset
 
 	for itemIndex < len(p.items) {
 		p.items[itemIndex].parse(c)
 		if !c.matchLast {
 			if currentCount < p.ranges[itemIndex][0] {
-				if c.failOffset > initialFailOffset && c.failingParser == nil {
+				if c.failingParser == nil &&
-					if p.commitType()&userDefined != 0 && p.commitType()&Whitespace == 0 {
+					p.commitType()&userDefined != 0 &&
-						// println("recording sequence failure", p.name, from, c.failOffset)
+					p.commitType()&Whitespace == 0 {
-						c.failingParser = p
+					c.failingParser = p
-					}
 				}
 
 				c.fail(from)
@@ -352,7 +350,7 @@ func (p *sequenceParser) parse(c *context) {
 		}
 	}
 
-	if to >= c.failOffset {
+	if to > c.failOffset {
 		c.failOffset = -1
 		c.failingParser = nil
 	}

syntax.go

@@ -159,7 +159,7 @@ func intsContain(is []int, i int) bool {
 
 func (pe *ParseError) Error() string {
 	return fmt.Sprintf(
-		"%s:%d:%d:failed to parse input, expecting: %s",
+		"%s:%d:%d:parse failed, parsing: %s",
 		pe.Input,
 		pe.Line+1,
 		pe.Column+1,