[go] text/template: remove concurrency from scanner
Gerrit Bot (Gerrit)
Gerrit Bot has uploaded this change for review.
text/template: remove concurrency from scanner
Makes the scanner in text/template fully synchronous. Replaces the unbuffered item channel with a slice implementing a simple queue.
Fixes #53261
Change-Id: I6d15354b5a101201e340da55e7995bfba117d33d
GitHub-Last-Rev: c0581a0f654acdec34c102fa52c1953e1a90194d
GitHub-Pull-Request: golang/go#53405
---
M src/text/template/parse/lex.go
M src/text/template/parse/lex_test.go
M src/text/template/parse/parse.go
3 files changed, 36 insertions(+), 44 deletions(-)
diff --git a/src/text/template/parse/lex.go b/src/text/template/parse/lex.go
index 29403dd..de07111 100644
--- a/src/text/template/parse/lex.go
+++ b/src/text/template/parse/lex.go
@@ -111,20 +111,20 @@
// lexer holds the state of the scanner.
type lexer struct {
- name string // the name of the input; used only for error reports
- input string // the string being scanned
- leftDelim string // start of action
- rightDelim string // end of action
- emitComment bool // emit itemComment tokens.
- pos Pos // current position in the input
- start Pos // start position of this item
- atEOF bool // we have hit the end of input and returned eof
- items chan item // channel of scanned items
- parenDepth int // nesting depth of ( ) exprs
- line int // 1+number of newlines seen
- startLine int // start line of this item
- breakOK bool // break keyword allowed
- continueOK bool // continue keyword allowed
+ name string // the name of the input; used only for error reports
+ input string // the string being scanned
+ leftDelim string // start of action
+ rightDelim string // end of action
+ emitComment bool // emit itemComment tokens.
+ pos Pos // current position in the input
+ start Pos // start position of this item
+ atEOF bool // we have hit the end of input and returned eof
+ items []item // queue of scanned items
+ parenDepth int // nesting depth of ( ) exprs
+ line int // 1+number of newlines seen
+ startLine int // start line of this item
+ breakOK bool // break keyword allowed
+ continueOK bool // continue keyword allowed
}
// next returns the next rune in the input.
@@ -162,7 +162,7 @@
// emit passes an item back to the client.
func (l *lexer) emit(t itemType) {
- l.items <- item{t, l.start, l.input[l.start:l.pos], l.startLine}
+ l.items = append(l.items, item{t, l.start, l.input[l.start:l.pos], l.startLine})
l.start = l.pos
l.startLine = l.line
}
@@ -193,21 +193,16 @@
// errorf 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 ...any) stateFn {
- l.items <- item{itemError, l.start, fmt.Sprintf(format, args...), l.startLine}
+ l.items = append(l.items, item{itemError, l.start, fmt.Sprintf(format, args...), l.startLine})
return nil
}
// nextItem returns the next item from the input.
// Called by the parser, not in the lexing goroutine.
func (l *lexer) nextItem() item {
- return <-l.items
-}
-
-// drain drains the output so the lexing goroutine will exit.
-// Called by the parser, not in the lexing goroutine.
-func (l *lexer) drain() {
- for range l.items {
- }
+ it := l.items[0]
+ l.items = append(l.items[:0], l.items[1:]...)
+ return it
}
// lex creates a new scanner for the input string.
@@ -226,11 +221,11 @@
emitComment: emitComment,
breakOK: breakOK,
continueOK: continueOK,
- items: make(chan item),
+ items: make([]item, 0, len(input)),
line: 1,
startLine: 1,
}
- go l.run()
+ l.run()
return l
}
@@ -239,7 +234,6 @@
for state := lexText; state != nil; {
state = state(l)
}
- close(l.items)
}
// state functions
diff --git a/src/text/template/parse/lex_test.go b/src/text/template/parse/lex_test.go
index c5f4296..ed199fe 100644
--- a/src/text/template/parse/lex_test.go
+++ b/src/text/template/parse/lex_test.go
@@ -546,22 +546,6 @@
}
}
-// Test that an error shuts down the lexing goroutine.
-func TestShutdown(t *testing.T) {
- // We need to duplicate template.Parse here to hold on to the lexer.
- const text = "erroneous{{define}}{{else}}1234"
- lexer := lex("foo", text, "{{", "}}", false, true, true)
- _, err := New("root").parseLexer(lexer)
- if err == nil {
- t.Fatalf("expected error")
- }
- // The error should have drained the input. Therefore, the lexer should be shut down.
- token, ok := <-lexer.items
- if ok {
- t.Fatalf("input was not drained; got %v", token)
- }
-}
-
// parseLexer is a local version of parse that lets us pass in the lexer instead of building it.
// We expect an error, so the tree set and funcs list are explicitly nil.
func (t *Tree) parseLexer(lex *lexer) (tree *Tree, err error) {
diff --git a/src/text/template/parse/parse.go b/src/text/template/parse/parse.go
index 00c258a..0485b11 100644
--- a/src/text/template/parse/parse.go
+++ b/src/text/template/parse/parse.go
@@ -210,7 +210,6 @@
panic(e)
}
if t != nil {
- t.lex.drain()
t.stopParse()
}
*errp = e.(error)
To view, visit change 412574. To unsubscribe, or for help writing mail filters, visit settings.
Gopher Robot (Gerrit)
Congratulations on opening your first change. Thank you for your contribution!
Next steps:
A maintainer will review your change and provide feedback. See
https://go.dev/doc/contribute#review for more info and tips to get your
patch through code review.
Most changes in the Go project go through a few rounds of revision. This can be
surprising to people new to the project. The careful, iterative review process
is our way of helping mentor contributors and ensuring that their contributions
have a lasting impact.
During May-July and Nov-Jan the Go project is in a code freeze, during which
little code gets reviewed or merged. If a reviewer responds with a comment like
R=go1.11 or adds a tag like "wait-release", it means that this CL will be
reviewed as part of the next development cycle. See https://go.dev/s/release
for more details.
Rob Pike (Gerrit)
1 comment:
Patchset:
This is essentially just recreating the channel. Yes, it removes the concurrency but by managing the slice it's not really doing what I think is the best solution.
We can land this if you want, but it's not the fix I was after. I assigned the issue to myself because I had a particular solution in mind, and will land it soon after the tree opens for the next cycle.
Please remove the "Fixes #53261" from the CL description, or change it to "Updates". Technically it does fix the core problem, but I want to keep that issue open.
To view, visit change 412574. To unsubscribe, or for help writing mail filters, visit settings.
Gerrit Bot (Gerrit)
Gerrit Bot uploaded patch set #2 to this change.
text/template: remove concurrency from scanner
Makes the scanner in text/template fully synchronous. Replaces the unbuffered item channel with a slice implementing a simple queue.
Change-Id: I6d15354b5a101201e340da55e7995bfba117d33d
GitHub-Last-Rev: c0581a0f654acdec34c102fa52c1953e1a90194d
GitHub-Pull-Request: golang/go#53405
---
M src/text/template/parse/lex.go
M src/text/template/parse/lex_test.go
M src/text/template/parse/parse.go
3 files changed, 34 insertions(+), 44 deletions(-)
To view, visit change 412574. To unsubscribe, or for help writing mail filters, visit settings.
Gopher Robot (Gerrit)
Gopher Robot abandoned this change.
To view, visit change 412574. To unsubscribe, or for help writing mail filters, visit settings.
Eli Shafer (Gerrit)
1 comment:
Patchset:
Thanks for the response! Obviously, this is my first attempt at open source -- I didn't realize this was an issue that you had self-assigned.
I'll go ahead and remove the "Fixes #53261", and if you have a different fix in mind, I don't necessarily think there is a reason to merge this versus just waiting for your fix.
To view, visit change 412574. To unsubscribe, or for help writing mail filters, visit settings.