From: Rob Pike Date: Mon, 30 Jul 2012 22:11:20 +0000 (-0700) Subject: text/template/parse: fix data race X-Git-Tag: go1.1rc2~2740 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=20d9fd3ae18bd5f80c3c0f8f424ebd9a72b6788a;p=gostls13.git text/template/parse: fix data race The situation only affects diagnostics but is easy to fix. When computing lineNumber, use the position of the last item returned by nextItem rather than the current state of the lexer. This is internal only and does not affect the API. Fixes #3886. R=golang-dev, iant CC=golang-dev https://golang.org/cl/6445061 --- diff --git a/src/pkg/text/template/parse/lex.go b/src/pkg/text/template/parse/lex.go index 62bf6d2009..dcf30f12e7 100644 --- a/src/pkg/text/template/parse/lex.go +++ b/src/pkg/text/template/parse/lex.go @@ -13,8 +13,9 @@ import ( // item represents a token or text string returned from the scanner. type item struct { - typ itemType - val string + typ itemType // The type of this item. + pos int // The starting position, in bytes, of this item in the input string. + val string // The value of this item. } func (i item) String() string { @@ -127,6 +128,7 @@ type lexer struct { pos int // current position in the input. start int // start position of this item. width int // width of last rune read from input. + lastPos int // position of nost recent item returned by nextItem items chan item // channel of scanned items. } @@ -155,7 +157,7 @@ func (l *lexer) backup() { // emit passes an item back to the client. func (l *lexer) emit(t itemType) { - l.items <- item{t, l.input[l.start:l.pos]} + l.items <- item{t, l.start, l.input[l.start:l.pos]} l.start = l.pos } @@ -180,22 +182,25 @@ func (l *lexer) acceptRun(valid string) { l.backup() } -// lineNumber reports which line we're on. Doing it this way +// lineNumber reports which line we're on, based on the position of +// the previous item returned by nextItem. Doing it this way // means we don't have to worry about peek double counting. func (l *lexer) lineNumber() int { - return 1 + strings.Count(l.input[:l.pos], "\n") + return 1 + strings.Count(l.input[:l.lastPos], "\n") } // error returns an error token and terminates the scan by passing // back a nil pointer that will be the next state, terminating l.nextItem. func (l *lexer) errorf(format string, args ...interface{}) stateFn { - l.items <- item{itemError, fmt.Sprintf(format, args...)} + l.items <- item{itemError, l.start, fmt.Sprintf(format, args...)} return nil } // nextItem returns the next item from the input. func (l *lexer) nextItem() item { - return <-l.items + item := <-l.items + l.lastPos = item.pos + return item } // lex creates a new scanner for the input string. diff --git a/src/pkg/text/template/parse/lex_test.go b/src/pkg/text/template/parse/lex_test.go index 6ee1b47010..26d242b41f 100644 --- a/src/pkg/text/template/parse/lex_test.go +++ b/src/pkg/text/template/parse/lex_test.go @@ -5,7 +5,6 @@ package parse import ( - "reflect" "testing" ) @@ -16,31 +15,31 @@ type lexTest struct { } var ( - tEOF = item{itemEOF, ""} - tLeft = item{itemLeftDelim, "{{"} - tRight = item{itemRightDelim, "}}"} - tRange = item{itemRange, "range"} - tPipe = item{itemPipe, "|"} - tFor = item{itemIdentifier, "for"} - tQuote = item{itemString, `"abc \n\t\" "`} + tEOF = item{itemEOF, 0, ""} + tLeft = item{itemLeftDelim, 0, "{{"} + tRight = item{itemRightDelim, 0, "}}"} + tRange = item{itemRange, 0, "range"} + tPipe = item{itemPipe, 0, "|"} + tFor = item{itemIdentifier, 0, "for"} + tQuote = item{itemString, 0, `"abc \n\t\" "`} raw = "`" + `abc\n\t\" ` + "`" - tRawQuote = item{itemRawString, raw} + tRawQuote = item{itemRawString, 0, raw} ) var lexTests = []lexTest{ {"empty", "", []item{tEOF}}, - {"spaces", " \t\n", []item{{itemText, " \t\n"}, tEOF}}, - {"text", `now is the time`, []item{{itemText, "now is the time"}, tEOF}}, + {"spaces", " \t\n", []item{{itemText, 0, " \t\n"}, tEOF}}, + {"text", `now is the time`, []item{{itemText, 0, "now is the time"}, tEOF}}, {"text with comment", "hello-{{/* this is a comment */}}-world", []item{ - {itemText, "hello-"}, - {itemText, "-world"}, + {itemText, 0, "hello-"}, + {itemText, 0, "-world"}, tEOF, }}, {"punctuation", "{{,@%}}", []item{ tLeft, - {itemChar, ","}, - {itemChar, "@"}, - {itemChar, "%"}, + {itemChar, 0, ","}, + {itemChar, 0, "@"}, + {itemChar, 0, "%"}, tRight, tEOF, }}, @@ -50,139 +49,139 @@ var lexTests = []lexTest{ {"raw quote", "{{" + raw + "}}", []item{tLeft, tRawQuote, tRight, tEOF}}, {"numbers", "{{1 02 0x14 -7.2i 1e3 +1.2e-4 4.2i 1+2i}}", []item{ tLeft, - {itemNumber, "1"}, - {itemNumber, "02"}, - {itemNumber, "0x14"}, - {itemNumber, "-7.2i"}, - {itemNumber, "1e3"}, - {itemNumber, "+1.2e-4"}, - {itemNumber, "4.2i"}, - {itemComplex, "1+2i"}, + {itemNumber, 0, "1"}, + {itemNumber, 0, "02"}, + {itemNumber, 0, "0x14"}, + {itemNumber, 0, "-7.2i"}, + {itemNumber, 0, "1e3"}, + {itemNumber, 0, "+1.2e-4"}, + {itemNumber, 0, "4.2i"}, + {itemComplex, 0, "1+2i"}, tRight, tEOF, }}, {"characters", `{{'a' '\n' '\'' '\\' '\u00FF' '\xFF' '本'}}`, []item{ tLeft, - {itemCharConstant, `'a'`}, - {itemCharConstant, `'\n'`}, - {itemCharConstant, `'\''`}, - {itemCharConstant, `'\\'`}, - {itemCharConstant, `'\u00FF'`}, - {itemCharConstant, `'\xFF'`}, - {itemCharConstant, `'本'`}, + {itemCharConstant, 0, `'a'`}, + {itemCharConstant, 0, `'\n'`}, + {itemCharConstant, 0, `'\''`}, + {itemCharConstant, 0, `'\\'`}, + {itemCharConstant, 0, `'\u00FF'`}, + {itemCharConstant, 0, `'\xFF'`}, + {itemCharConstant, 0, `'本'`}, tRight, tEOF, }}, {"bools", "{{true false}}", []item{ tLeft, - {itemBool, "true"}, - {itemBool, "false"}, + {itemBool, 0, "true"}, + {itemBool, 0, "false"}, tRight, tEOF, }}, {"dot", "{{.}}", []item{ tLeft, - {itemDot, "."}, + {itemDot, 0, "."}, tRight, tEOF, }}, {"dots", "{{.x . .2 .x.y}}", []item{ tLeft, - {itemField, ".x"}, - {itemDot, "."}, - {itemNumber, ".2"}, - {itemField, ".x.y"}, + {itemField, 0, ".x"}, + {itemDot, 0, "."}, + {itemNumber, 0, ".2"}, + {itemField, 0, ".x.y"}, tRight, tEOF, }}, {"keywords", "{{range if else end with}}", []item{ tLeft, - {itemRange, "range"}, - {itemIf, "if"}, - {itemElse, "else"}, - {itemEnd, "end"}, - {itemWith, "with"}, + {itemRange, 0, "range"}, + {itemIf, 0, "if"}, + {itemElse, 0, "else"}, + {itemEnd, 0, "end"}, + {itemWith, 0, "with"}, tRight, tEOF, }}, {"variables", "{{$c := printf $ $hello $23 $ $var.Field .Method}}", []item{ tLeft, - {itemVariable, "$c"}, - {itemColonEquals, ":="}, - {itemIdentifier, "printf"}, - {itemVariable, "$"}, - {itemVariable, "$hello"}, - {itemVariable, "$23"}, - {itemVariable, "$"}, - {itemVariable, "$var.Field"}, - {itemField, ".Method"}, + {itemVariable, 0, "$c"}, + {itemColonEquals, 0, ":="}, + {itemIdentifier, 0, "printf"}, + {itemVariable, 0, "$"}, + {itemVariable, 0, "$hello"}, + {itemVariable, 0, "$23"}, + {itemVariable, 0, "$"}, + {itemVariable, 0, "$var.Field"}, + {itemField, 0, ".Method"}, tRight, tEOF, }}, {"pipeline", `intro {{echo hi 1.2 |noargs|args 1 "hi"}} outro`, []item{ - {itemText, "intro "}, + {itemText, 0, "intro "}, tLeft, - {itemIdentifier, "echo"}, - {itemIdentifier, "hi"}, - {itemNumber, "1.2"}, + {itemIdentifier, 0, "echo"}, + {itemIdentifier, 0, "hi"}, + {itemNumber, 0, "1.2"}, tPipe, - {itemIdentifier, "noargs"}, + {itemIdentifier, 0, "noargs"}, tPipe, - {itemIdentifier, "args"}, - {itemNumber, "1"}, - {itemString, `"hi"`}, + {itemIdentifier, 0, "args"}, + {itemNumber, 0, "1"}, + {itemString, 0, `"hi"`}, tRight, - {itemText, " outro"}, + {itemText, 0, " outro"}, tEOF, }}, {"declaration", "{{$v := 3}}", []item{ tLeft, - {itemVariable, "$v"}, - {itemColonEquals, ":="}, - {itemNumber, "3"}, + {itemVariable, 0, "$v"}, + {itemColonEquals, 0, ":="}, + {itemNumber, 0, "3"}, tRight, tEOF, }}, {"2 declarations", "{{$v , $w := 3}}", []item{ tLeft, - {itemVariable, "$v"}, - {itemChar, ","}, - {itemVariable, "$w"}, - {itemColonEquals, ":="}, - {itemNumber, "3"}, + {itemVariable, 0, "$v"}, + {itemChar, 0, ","}, + {itemVariable, 0, "$w"}, + {itemColonEquals, 0, ":="}, + {itemNumber, 0, "3"}, tRight, tEOF, }}, // errors {"badchar", "#{{\x01}}", []item{ - {itemText, "#"}, + {itemText, 0, "#"}, tLeft, - {itemError, "unrecognized character in action: U+0001"}, + {itemError, 0, "unrecognized character in action: U+0001"}, }}, {"unclosed action", "{{\n}}", []item{ tLeft, - {itemError, "unclosed action"}, + {itemError, 0, "unclosed action"}, }}, {"EOF in action", "{{range", []item{ tLeft, tRange, - {itemError, "unclosed action"}, + {itemError, 0, "unclosed action"}, }}, {"unclosed quote", "{{\"\n\"}}", []item{ tLeft, - {itemError, "unterminated quoted string"}, + {itemError, 0, "unterminated quoted string"}, }}, {"unclosed raw quote", "{{`xx\n`}}", []item{ tLeft, - {itemError, "unterminated raw quoted string"}, + {itemError, 0, "unterminated raw quoted string"}, }}, {"unclosed char constant", "{{'\n}}", []item{ tLeft, - {itemError, "unterminated character constant"}, + {itemError, 0, "unterminated character constant"}, }}, {"bad number", "{{3k}}", []item{ tLeft, - {itemError, `bad number syntax: "3k"`}, + {itemError, 0, `bad number syntax: "3k"`}, }}, // Fixed bugs @@ -213,10 +212,28 @@ func collect(t *lexTest, left, right string) (items []item) { return } +func equal(i1, i2 []item, checkPos bool) bool { + if len(i1) != len(i2) { + return false + } + for k := range i1 { + if i1[k].typ != i2[k].typ { + return false + } + if i1[k].val != i2[k].val { + return false + } + if checkPos && i1[k].pos != i2[k].pos { + return false + } + } + return true +} + func TestLex(t *testing.T) { for _, test := range lexTests { items := collect(&test, "", "") - if !reflect.DeepEqual(items, test.items) { + if !equal(items, test.items, false) { t.Errorf("%s: got\n\t%v\nexpected\n\t%v", test.name, items, test.items) } } @@ -226,13 +243,13 @@ func TestLex(t *testing.T) { var lexDelimTests = []lexTest{ {"punctuation", "$$,@%{{}}@@", []item{ tLeftDelim, - {itemChar, ","}, - {itemChar, "@"}, - {itemChar, "%"}, - {itemChar, "{"}, - {itemChar, "{"}, - {itemChar, "}"}, - {itemChar, "}"}, + {itemChar, 0, ","}, + {itemChar, 0, "@"}, + {itemChar, 0, "%"}, + {itemChar, 0, "{"}, + {itemChar, 0, "{"}, + {itemChar, 0, "}"}, + {itemChar, 0, "}"}, tRightDelim, tEOF, }}, @@ -243,15 +260,57 @@ var lexDelimTests = []lexTest{ } var ( - tLeftDelim = item{itemLeftDelim, "$$"} - tRightDelim = item{itemRightDelim, "@@"} + tLeftDelim = item{itemLeftDelim, 0, "$$"} + tRightDelim = item{itemRightDelim, 0, "@@"} ) func TestDelims(t *testing.T) { for _, test := range lexDelimTests { items := collect(&test, "$$", "@@") - if !reflect.DeepEqual(items, test.items) { + if !equal(items, test.items, false) { + t.Errorf("%s: got\n\t%v\nexpected\n\t%v", test.name, items, test.items) + } + } +} + +var lexPosTests = []lexTest{ + {"empty", "", []item{tEOF}}, + {"punctuation", "{{,@%#}}", []item{ + {itemLeftDelim, 0, "{{"}, + {itemChar, 2, ","}, + {itemChar, 3, "@"}, + {itemChar, 4, "%"}, + {itemChar, 5, "#"}, + {itemRightDelim, 6, "}}"}, + {itemEOF, 8, ""}, + }}, + {"sample", "0123{{hello}}xyz", []item{ + {itemText, 0, "0123"}, + {itemLeftDelim, 4, "{{"}, + {itemIdentifier, 6, "hello"}, + {itemRightDelim, 11, "}}"}, + {itemText, 13, "xyz"}, + {itemEOF, 16, ""}, + }}, +} + +// The other tests don't check position, to make the test cases easier to construct. +// This one does. +func TestPos(t *testing.T) { + for _, test := range lexPosTests { + items := collect(&test, "", "") + if !equal(items, test.items, true) { t.Errorf("%s: got\n\t%v\nexpected\n\t%v", test.name, items, test.items) + if len(items) == len(test.items) { + // Detailed print; avoid item.String() to expose the position value. + for i := range items { + if !equal(items[i:i+1], test.items[i:i+1], true) { + i1 := items[i] + i2 := test.items[i] + t.Errorf("\t#%d: got {%v %d %q} expected {%v %d %q}", i, i1.typ, i1.pos, i1.val, i2.typ, i2.pos, i2.val) + } + } + } } } }