Skip to content

Commit

Permalink
text/template: implement short-circuit and, or
Browse files Browse the repository at this point in the history
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 golang#31103.

Change-Id: Ia43d4a9a6b0ab814f2dd3471ebaca3e7bb1505cf
Reviewed-on: https://go-review.googlesource.com/c/go/+/321490
Trust: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
  • Loading branch information
rsc committed Sep 21, 2021
1 parent 39e08c6 commit 7d67f8d
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 33 deletions.
12 changes: 7 additions & 5 deletions src/text/template/doc.go
Expand Up @@ -307,9 +307,10 @@ Predefined global functions are named as follows.
and
Returns the boolean AND of its arguments by returning the
first empty argument or the last argument, that is,
"and x y" behaves as "if x then y else x". All the
arguments are evaluated.
first empty argument or the last argument. That is,
"and x y" behaves as "if x then y else x."
Evaluation proceeds through the arguments left to right
and returns when the result is determined.
call
Returns the result of calling the first argument, which
must be a function, with the remaining arguments as parameters.
Expand Down Expand Up @@ -344,8 +345,9 @@ Predefined global functions are named as follows.
or
Returns the boolean OR of its arguments by returning the
first non-empty argument or the last argument, that is,
"or x y" behaves as "if x then x else y". All the
arguments are evaluated.
"or x y" behaves as "if x then x else y".
Evaluation proceeds through the arguments left to right
and returns when the result is determined.
print
An alias for fmt.Sprint
printf
Expand Down
22 changes: 18 additions & 4 deletions src/text/template/exec.go
Expand Up @@ -572,11 +572,11 @@ func (s *state) evalFieldChain(dot, receiver reflect.Value, node parse.Node, ide
func (s *state) evalFunction(dot reflect.Value, node *parse.IdentifierNode, cmd parse.Node, args []parse.Node, final reflect.Value) reflect.Value {
s.at(node)
name := node.Ident
function, ok := findFunction(name, s.tmpl)
function, isBuiltin, ok := findFunction(name, s.tmpl)
if !ok {
s.errorf("%q is not a defined function", name)
}
return s.evalCall(dot, function, cmd, name, args, final)
return s.evalCall(dot, function, isBuiltin, cmd, name, args, final)
}

// evalField evaluates an expression like (.Field) or (.Field arg1 arg2).
Expand Down Expand Up @@ -605,7 +605,7 @@ func (s *state) evalField(dot reflect.Value, fieldName string, node parse.Node,
ptr = ptr.Addr()
}
if method := ptr.MethodByName(fieldName); method.IsValid() {
return s.evalCall(dot, method, node, fieldName, args, final)
return s.evalCall(dot, method, false, node, fieldName, args, final)
}
hasArgs := len(args) > 1 || final != missingVal
// It's not a method; must be a field of a struct or an element of a map.
Expand Down Expand Up @@ -669,7 +669,7 @@ var (
// evalCall executes a function or method call. If it's a method, fun already has the receiver bound, so
// it looks just like a function call. The arg list, if non-nil, includes (in the manner of the shell), arg[0]
// as the function itself.
func (s *state) evalCall(dot, fun reflect.Value, node parse.Node, name string, args []parse.Node, final reflect.Value) reflect.Value {
func (s *state) evalCall(dot, fun reflect.Value, isBuiltin bool, node parse.Node, name string, args []parse.Node, final reflect.Value) reflect.Value {
if args != nil {
args = args[1:] // Zeroth arg is function name/node; not passed to function.
}
Expand All @@ -691,6 +691,20 @@ func (s *state) evalCall(dot, fun reflect.Value, node parse.Node, name string, a
// TODO: This could still be a confusing error; maybe goodFunc should provide info.
s.errorf("can't call method/function %q with %d results", name, typ.NumOut())
}

// Special case for builtin and/or, which short-circuit.
if isBuiltin && (name == "and" || name == "or") {
argType := typ.In(0)
var v reflect.Value
for _, arg := range args {
v = s.evalArg(dot, argType, arg).Interface().(reflect.Value)
if truth(v) == (name == "or") {
break
}
}
return v
}

// Build the arg list.
argv := make([]reflect.Value, numIn)
// Args must be evaluated. Fixed args first.
Expand Down
5 changes: 5 additions & 0 deletions src/text/template/exec_test.go
Expand Up @@ -481,6 +481,10 @@ var execTests = []execTest{
{"not", "{{not true}} {{not false}}", "false true", nil, true},
{"and", "{{and false 0}} {{and 1 0}} {{and 0 true}} {{and 1 1}}", "false 0 0 1", nil, true},
{"or", "{{or 0 0}} {{or 1 0}} {{or 0 true}} {{or 1 1}}", "0 1 true 1", nil, true},
{"or short-circuit", "{{or 0 1 (die)}}", "1", nil, true},
{"and short-circuit", "{{and 1 0 (die)}}", "0", nil, true},
{"or short-circuit2", "{{or 0 0 (die)}}", "", nil, false},
{"and short-circuit2", "{{and 1 1 (die)}}", "", nil, false},
{"boolean if", "{{if and true 1 `hi`}}TRUE{{else}}FALSE{{end}}", "TRUE", tVal, true},
{"boolean if not", "{{if and true 1 `hi` | not}}TRUE{{else}}FALSE{{end}}", "FALSE", nil, true},

Expand Down Expand Up @@ -764,6 +768,7 @@ func testExecute(execTests []execTest, template *Template, t *testing.T) {
"add": add,
"count": count,
"dddArg": dddArg,
"die": func() bool { panic("die") },
"echo": echo,
"makemap": makemap,
"mapOfThree": mapOfThree,
Expand Down
30 changes: 6 additions & 24 deletions src/text/template/funcs.go
Expand Up @@ -139,18 +139,18 @@ func goodName(name string) bool {
}

// findFunction looks for a function in the template, and global map.
func findFunction(name string, tmpl *Template) (reflect.Value, bool) {
func findFunction(name string, tmpl *Template) (v reflect.Value, isBuiltin, ok bool) {
if tmpl != nil && tmpl.common != nil {
tmpl.muFuncs.RLock()
defer tmpl.muFuncs.RUnlock()
if fn := tmpl.execFuncs[name]; fn.IsValid() {
return fn, true
return fn, false, true
}
}
if fn := builtinFuncs()[name]; fn.IsValid() {
return fn, true
return fn, true, true
}
return reflect.Value{}, false
return reflect.Value{}, false, false
}

// prepareArg checks if value can be used as an argument of type argType, and
Expand Down Expand Up @@ -382,31 +382,13 @@ func truth(arg reflect.Value) bool {
// and computes the Boolean AND of its arguments, returning
// the first false argument it encounters, or the last argument.
func and(arg0 reflect.Value, args ...reflect.Value) reflect.Value {
if !truth(arg0) {
return arg0
}
for i := range args {
arg0 = args[i]
if !truth(arg0) {
break
}
}
return arg0
panic("unreachable") // implemented as a special case in evalCall
}

// or computes the Boolean OR of its arguments, returning
// the first true argument it encounters, or the last argument.
func or(arg0 reflect.Value, args ...reflect.Value) reflect.Value {
if truth(arg0) {
return arg0
}
for i := range args {
arg0 = args[i]
if truth(arg0) {
break
}
}
return arg0
panic("unreachable") // implemented as a special case in evalCall
}

// not returns the Boolean negation of its argument.
Expand Down

0 comments on commit 7d67f8d

Please sign in to comment.