Skip to content

Commit 7d67f8d

Browse files
committed
text/template: implement short-circuit and, or
Making the builtin and and or functions use short-circuit evaluation was accepted as a proposal in April 2019, but we never got around to implementing it. Do that. Fixes #31103. Change-Id: Ia43d4a9a6b0ab814f2dd3471ebaca3e7bb1505cf Reviewed-on: https://go-review.googlesource.com/c/go/+/321490 Trust: Russ Cox <[email protected]> Run-TryBot: Russ Cox <[email protected]> TryBot-Result: Go Bot <[email protected]> Reviewed-by: Rob Pike <[email protected]>
1 parent 39e08c6 commit 7d67f8d

File tree

4 files changed

+36
-33
lines changed

4 files changed

+36
-33
lines changed

src/text/template/doc.go

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -307,9 +307,10 @@ Predefined global functions are named as follows.
307307
308308
and
309309
Returns the boolean AND of its arguments by returning the
310-
first empty argument or the last argument, that is,
311-
"and x y" behaves as "if x then y else x". All the
312-
arguments are evaluated.
310+
first empty argument or the last argument. That is,
311+
"and x y" behaves as "if x then y else x."
312+
Evaluation proceeds through the arguments left to right
313+
and returns when the result is determined.
313314
call
314315
Returns the result of calling the first argument, which
315316
must be a function, with the remaining arguments as parameters.
@@ -344,8 +345,9 @@ Predefined global functions are named as follows.
344345
or
345346
Returns the boolean OR of its arguments by returning the
346347
first non-empty argument or the last argument, that is,
347-
"or x y" behaves as "if x then x else y". All the
348-
arguments are evaluated.
348+
"or x y" behaves as "if x then x else y".
349+
Evaluation proceeds through the arguments left to right
350+
and returns when the result is determined.
349351
print
350352
An alias for fmt.Sprint
351353
printf

src/text/template/exec.go

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -572,11 +572,11 @@ func (s *state) evalFieldChain(dot, receiver reflect.Value, node parse.Node, ide
572572
func (s *state) evalFunction(dot reflect.Value, node *parse.IdentifierNode, cmd parse.Node, args []parse.Node, final reflect.Value) reflect.Value {
573573
s.at(node)
574574
name := node.Ident
575-
function, ok := findFunction(name, s.tmpl)
575+
function, isBuiltin, ok := findFunction(name, s.tmpl)
576576
if !ok {
577577
s.errorf("%q is not a defined function", name)
578578
}
579-
return s.evalCall(dot, function, cmd, name, args, final)
579+
return s.evalCall(dot, function, isBuiltin, cmd, name, args, final)
580580
}
581581

582582
// evalField evaluates an expression like (.Field) or (.Field arg1 arg2).
@@ -605,7 +605,7 @@ func (s *state) evalField(dot reflect.Value, fieldName string, node parse.Node,
605605
ptr = ptr.Addr()
606606
}
607607
if method := ptr.MethodByName(fieldName); method.IsValid() {
608-
return s.evalCall(dot, method, node, fieldName, args, final)
608+
return s.evalCall(dot, method, false, node, fieldName, args, final)
609609
}
610610
hasArgs := len(args) > 1 || final != missingVal
611611
// It's not a method; must be a field of a struct or an element of a map.
@@ -669,7 +669,7 @@ var (
669669
// evalCall executes a function or method call. If it's a method, fun already has the receiver bound, so
670670
// it looks just like a function call. The arg list, if non-nil, includes (in the manner of the shell), arg[0]
671671
// as the function itself.
672-
func (s *state) evalCall(dot, fun reflect.Value, node parse.Node, name string, args []parse.Node, final reflect.Value) reflect.Value {
672+
func (s *state) evalCall(dot, fun reflect.Value, isBuiltin bool, node parse.Node, name string, args []parse.Node, final reflect.Value) reflect.Value {
673673
if args != nil {
674674
args = args[1:] // Zeroth arg is function name/node; not passed to function.
675675
}
@@ -691,6 +691,20 @@ func (s *state) evalCall(dot, fun reflect.Value, node parse.Node, name string, a
691691
// TODO: This could still be a confusing error; maybe goodFunc should provide info.
692692
s.errorf("can't call method/function %q with %d results", name, typ.NumOut())
693693
}
694+
695+
// Special case for builtin and/or, which short-circuit.
696+
if isBuiltin && (name == "and" || name == "or") {
697+
argType := typ.In(0)
698+
var v reflect.Value
699+
for _, arg := range args {
700+
v = s.evalArg(dot, argType, arg).Interface().(reflect.Value)
701+
if truth(v) == (name == "or") {
702+
break
703+
}
704+
}
705+
return v
706+
}
707+
694708
// Build the arg list.
695709
argv := make([]reflect.Value, numIn)
696710
// Args must be evaluated. Fixed args first.

src/text/template/exec_test.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -481,6 +481,10 @@ var execTests = []execTest{
481481
{"not", "{{not true}} {{not false}}", "false true", nil, true},
482482
{"and", "{{and false 0}} {{and 1 0}} {{and 0 true}} {{and 1 1}}", "false 0 0 1", nil, true},
483483
{"or", "{{or 0 0}} {{or 1 0}} {{or 0 true}} {{or 1 1}}", "0 1 true 1", nil, true},
484+
{"or short-circuit", "{{or 0 1 (die)}}", "1", nil, true},
485+
{"and short-circuit", "{{and 1 0 (die)}}", "0", nil, true},
486+
{"or short-circuit2", "{{or 0 0 (die)}}", "", nil, false},
487+
{"and short-circuit2", "{{and 1 1 (die)}}", "", nil, false},
484488
{"boolean if", "{{if and true 1 `hi`}}TRUE{{else}}FALSE{{end}}", "TRUE", tVal, true},
485489
{"boolean if not", "{{if and true 1 `hi` | not}}TRUE{{else}}FALSE{{end}}", "FALSE", nil, true},
486490

@@ -764,6 +768,7 @@ func testExecute(execTests []execTest, template *Template, t *testing.T) {
764768
"add": add,
765769
"count": count,
766770
"dddArg": dddArg,
771+
"die": func() bool { panic("die") },
767772
"echo": echo,
768773
"makemap": makemap,
769774
"mapOfThree": mapOfThree,

src/text/template/funcs.go

Lines changed: 6 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -139,18 +139,18 @@ func goodName(name string) bool {
139139
}
140140

141141
// findFunction looks for a function in the template, and global map.
142-
func findFunction(name string, tmpl *Template) (reflect.Value, bool) {
142+
func findFunction(name string, tmpl *Template) (v reflect.Value, isBuiltin, ok bool) {
143143
if tmpl != nil && tmpl.common != nil {
144144
tmpl.muFuncs.RLock()
145145
defer tmpl.muFuncs.RUnlock()
146146
if fn := tmpl.execFuncs[name]; fn.IsValid() {
147-
return fn, true
147+
return fn, false, true
148148
}
149149
}
150150
if fn := builtinFuncs()[name]; fn.IsValid() {
151-
return fn, true
151+
return fn, true, true
152152
}
153-
return reflect.Value{}, false
153+
return reflect.Value{}, false, false
154154
}
155155

156156
// prepareArg checks if value can be used as an argument of type argType, and
@@ -382,31 +382,13 @@ func truth(arg reflect.Value) bool {
382382
// and computes the Boolean AND of its arguments, returning
383383
// the first false argument it encounters, or the last argument.
384384
func and(arg0 reflect.Value, args ...reflect.Value) reflect.Value {
385-
if !truth(arg0) {
386-
return arg0
387-
}
388-
for i := range args {
389-
arg0 = args[i]
390-
if !truth(arg0) {
391-
break
392-
}
393-
}
394-
return arg0
385+
panic("unreachable") // implemented as a special case in evalCall
395386
}
396387

397388
// or computes the Boolean OR of its arguments, returning
398389
// the first true argument it encounters, or the last argument.
399390
func or(arg0 reflect.Value, args ...reflect.Value) reflect.Value {
400-
if truth(arg0) {
401-
return arg0
402-
}
403-
for i := range args {
404-
arg0 = args[i]
405-
if truth(arg0) {
406-
break
407-
}
408-
}
409-
return arg0
391+
panic("unreachable") // implemented as a special case in evalCall
410392
}
411393

412394
// not returns the Boolean negation of its argument.

0 commit comments

Comments
 (0)