Clean up some notes and refer to generations instead of stack

Author: paulsmith

src/text/template/exec.go

@@ -33,10 +33,10 @@ func initMaxExecDepth() int {
 type state struct {
 	tmpl  *Template
 	wr    io.Writer
-	node  parse.Node // current node, for errors
-	vars  []variable // push-down stack of variable values.
-	depth int        // the height of the stack of executing templates.
-	scope map[string]int
+	node  parse.Node     // current node, for errors
+	vars  []variable     // push-down stack of variable values.
+	depth int            // the height of the stack of executing templates.
+	level map[string]int // map from template name to current level.
 }
 
 // variable holds the dynamic value of a variable such as $, $x etc.
@@ -211,7 +211,7 @@ func (t *Template) execute(wr io.Writer, data interface{}) (err error) {
 		tmpl:  t,
 		wr:    wr,
 		vars:  []variable{{"$", value}},
-		scope: make(map[string]int),
+		level: make(map[string]int),
 	}
 	if t.Tree == nil || t.Root == nil {
 		state.errorf("%q is an incomplete or empty template", t.Name())
@@ -403,12 +403,12 @@ func (s *state) walkRange(dot reflect.Value, r *parse.RangeNode) {
 
 func (s *state) walkTemplate(dot reflect.Value, t *parse.TemplateNode) {
 	s.at(t)
-	scope := s.scope[t.Name]
+	level := s.level[t.Name]
 	if t.Parent {
-		scope++
+		level++
 	}
-	s.scope[t.Name] = scope
-	tmpl := s.tmpl.LookupWithScope(t.Name, scope)
+	s.level[t.Name] = level
+	tmpl := s.tmpl.LookupByLevel(t.Name, level)
 	if tmpl == nil {
 		s.errorf("template %q not defined", t.Name)
 	}
@@ -423,11 +423,11 @@ func (s *state) walkTemplate(dot reflect.Value, t *parse.TemplateNode) {
 	// No dynamic scoping: template invocations inherit no variables.
 	newState.vars = []variable{{"$", dot}}
 	newState.walk(dot, tmpl.Root)
-	scope--
-	if scope < 0 { // TODO logical bug
-		scope = 0
+	level--
+	if level < 0 {
+		level = 0 // TODO: this could be masking a bug
 	}
-	newState.scope[t.Name] = scope // TODO: unclear if this makes sense here
+	newState.level[t.Name] = level
 }
 
 // Eval functions evaluate pipelines, commands, and their elements and extract

src/text/template/parse/lex_test.go

@@ -40,6 +40,7 @@ var itemName = map[itemType]string{
 	itemIf:       "if",
 	itemEnd:      "end",
 	itemNil:      "nil",
+	itemParent:   "parent",
 	itemRange:    "range",
 	itemTemplate: "template",
 	itemWith:     "with",

src/text/template/template.go

@@ -12,8 +12,11 @@ import (
 
 // common holds the information shared by related templates.
 type common struct {
-	tmpl   map[string][]*Template // Map from name to defined templates.
-	muTmpl sync.RWMutex           // protects tmpl
+	// Map from name to defined templates. There is a slice of templates,
+	// because there are generations of previously defined templates with the
+	// same name.
+	tmpl   map[string][]*Template
+	muTmpl sync.RWMutex // protects tmpl
 	option option
 	// We use two maps, one for parsing and one for execution.
 	// This separation makes the API cleaner since it doesn't
@@ -91,14 +94,14 @@ func (t *Template) Clone() (*Template, error) {
 	}
 	t.muTmpl.RLock()
 	defer t.muTmpl.RUnlock()
-	for k, stack := range t.tmpl {
+	for k, generations := range t.tmpl {
 		if k == t.name {
-			nt.tmpl[t.name] = append(nt.tmpl[t.name], nt) // TODO: double-check this - maybe should just be assigning new slice
+			nt.tmpl[t.name] = append(nt.tmpl[t.name], nt)
 			continue
 		}
 		// The associated templates share nt's common structure.
-		for i := range stack {
-			tmpl := stack[i].copy(nt.common)
+		for i := range generations {
+			tmpl := generations[i].copy(nt.common)
 			nt.tmpl[k] = append(nt.tmpl[k], tmpl)
 		}
 	}
@@ -152,8 +155,8 @@ func (t *Template) Templates() []*Template {
 	t.muTmpl.RLock()
 	defer t.muTmpl.RUnlock()
 	m := make([]*Template, 0, len(t.tmpl))
-	for _, stack := range t.tmpl {
-		m = append(m, stack[0]) // TODO: runtime assert there is at least one? invariant check
+	for _, generations := range t.tmpl {
+		m = append(m, generations[0])
 	}
 	return m
 }
@@ -196,20 +199,20 @@ func (t *Template) Lookup(name string) *Template {
 	return t.lookup(name, 0)
 }
 
-func (t *Template) LookupWithScope(name string, scope int) *Template {
+func (t *Template) LookupByLevel(name string, level int) *Template {
 	if t.common == nil {
 		return nil
 	}
 	t.muTmpl.RLock()
 	defer t.muTmpl.RUnlock()
-	return t.lookup(name, scope)
+	return t.lookup(name, level)
 }
 
-func (t *Template) lookup(name string, scope int) *Template {
-	if scope >= len(t.tmpl[name]) {
+func (t *Template) lookup(name string, level int) *Template {
+	if level < 0 || level >= len(t.tmpl[name]) {
 		return nil
 	}
-	return t.tmpl[name][scope] // TODO: invariant check: at least one
+	return t.tmpl[name][level]
 }
 
 // Parse parses text as a template body for t.
@@ -251,10 +254,17 @@ func (t *Template) associate(new *Template, tree *parse.Tree) bool {
 		// don't replace it with an empty template.
 		return false
 	}
-	stack := t.tmpl[new.name]
-	stack = append(stack, nil)
-	copy(stack[1:], stack[0:])
-	stack[0] = new
-	t.tmpl[new.name] = stack
+
+	// associating a template with the name of an existing one retains a
+	// reference to the original, pushing the new one to the front of a slice
+	// of previously defined templates, so that previously defined templates by
+	// that name can be accessed by indexing into the slice, each successive
+	// level being an older generation.
+	generations := t.tmpl[new.name]
+	generations = append(generations, nil)
+	copy(generations[1:], generations[0:])
+	generations[0] = new
+	t.tmpl[new.name] = generations
+
 	return true
 }