]> Cypherpunks repositories - gostls13.git/commitdiff
go/types, types2: remove argument "getter" use from Checker.builtins (cleanup)
authorRobert Griesemer <gri@golang.org>
Tue, 16 May 2023 21:15:07 +0000 (14:15 -0700)
committerGopher Robot <gobot@golang.org>
Thu, 18 May 2023 18:39:12 +0000 (18:39 +0000)
Check all arguments for validity once, in the beginning.
Conservatively replace arg(x, i) calls with *x = args[i].
Use y (2nd arguments) directly, w/o copying.
Remove unnecessary copies and slice creations in append.

Change-Id: I1e2891cba9658f5b3cdf897e81db2f690a99b16b
Reviewed-on: https://go-review.googlesource.com/c/go/+/495515
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>

src/cmd/compile/internal/types2/builtins.go
src/go/types/builtins.go

index 13736ec113b8b30dbe78aaa9b0a2d233564f2613..63b62a66d2090291585ff037caf7699dc3f1dd55 100644 (file)
@@ -39,24 +39,28 @@ func (check *Checker) builtin(x *operand, call *syntax.CallExpr, id builtinId) (
                check.hasCallOrRecv = false
        }
 
-       // determine actual arguments
-       var arg func(*operand, int) // TODO(gri) remove use of arg getter in favor of using xlist directly
-       nargs := len(call.ArgList)
+       // Evaluate arguments for built-ins that use ordinary (value) arguments.
+       // For built-ins with special argument handling (make, new, etc.),
+       // evaluation is done by the respective built-in code.
+       var args []*operand // not valid for _Make, _New, _Offsetof, _Trace
+       var nargs int
        switch id {
        default:
-               // make argument getter
-               xlist := check.exprList(call.ArgList)
-               arg = func(x *operand, i int) { *x = *xlist[i] }
-               nargs = len(xlist)
-               // evaluate first argument, if present
-               if nargs > 0 {
-                       arg(x, 0)
-                       if x.mode == invalid {
+               // check all arguments
+               args = check.exprList(call.ArgList)
+               nargs = len(args)
+               for _, a := range args {
+                       if a.mode == invalid {
                                return
                        }
                }
+               // first argument is always in x
+               if nargs > 0 {
+                       *x = *args[0]
+               }
        case _Make, _New, _Offsetof, _Trace:
                // arguments require special handling
+               nargs = len(call.ArgList)
        }
 
        // check argument count
@@ -103,21 +107,15 @@ func (check *Checker) builtin(x *operand, call *syntax.CallExpr, id builtinId) (
                        return
                }
 
-               // remember arguments that have been evaluated already
-               alist := []operand{*x}
-
                // spec: "As a special case, append also accepts a first argument assignable
                // to type []byte with a second argument of string type followed by ... .
                // This form appends the bytes of the string.
                if nargs == 2 && call.HasDots {
                        if ok, _ := x.assignableTo(check, NewSlice(universeByte), nil); ok {
-                               arg(x, 1)
-                               if x.mode == invalid {
-                                       return
-                               }
-                               if t := coreString(x.typ); t != nil && isString(t) {
+                               y := args[1]
+                               if t := coreString(y.typ); t != nil && isString(t) {
                                        if check.recordTypes() {
-                                               sig := makeSig(S, S, x.typ)
+                                               sig := makeSig(S, S, y.typ)
                                                sig.variadic = true
                                                check.recordBuiltinType(call.Fun, sig)
                                        }
@@ -125,25 +123,13 @@ func (check *Checker) builtin(x *operand, call *syntax.CallExpr, id builtinId) (
                                        x.typ = S
                                        break
                                }
-                               alist = append(alist, *x)
-                               // fallthrough
                        }
                }
 
                // check general case by creating custom signature
                sig := makeSig(S, S, NewSlice(T)) // []T required for variadic signature
                sig.variadic = true
-               var alist2 []*operand
-               // convert []operand to []*operand
-               for i := range alist {
-                       alist2 = append(alist2, &alist[i])
-               }
-               for i := len(alist); i < nargs; i++ {
-                       var x operand
-                       arg(&x, i)
-                       alist2 = append(alist2, &x)
-               }
-               check.arguments(call, sig, nil, nil, alist2, nil, nil) // discard result (we know the result type)
+               check.arguments(call, sig, nil, nil, args, nil, nil) // discard result (we know the result type)
                // ok to continue even if check.arguments reported errors
 
                x.mode = value
@@ -277,11 +263,7 @@ func (check *Checker) builtin(x *operand, call *syntax.CallExpr, id builtinId) (
 
        case _Complex:
                // complex(x, y floatT) complexT
-               var y operand
-               arg(&y, 1)
-               if y.mode == invalid {
-                       return
-               }
+               y := args[1]
 
                // convert or check untyped arguments
                d := 0
@@ -299,7 +281,7 @@ func (check *Checker) builtin(x *operand, call *syntax.CallExpr, id builtinId) (
                        check.convertUntyped(x, y.typ)
                case 2:
                        // only y is untyped => convert to type of x
-                       check.convertUntyped(&y, x.typ)
+                       check.convertUntyped(y, x.typ)
                case 3:
                        // x and y are untyped =>
                        // 1) if both are constants, convert them to untyped
@@ -316,10 +298,10 @@ func (check *Checker) builtin(x *operand, call *syntax.CallExpr, id builtinId) (
                                        }
                                }
                                toFloat(x)
-                               toFloat(&y)
+                               toFloat(y)
                        } else {
                                check.convertUntyped(x, Typ[Float64])
-                               check.convertUntyped(&y, Typ[Float64])
+                               check.convertUntyped(y, Typ[Float64])
                                // x and y should be invalid now, but be conservative
                                // and check below
                        }
@@ -373,11 +355,7 @@ func (check *Checker) builtin(x *operand, call *syntax.CallExpr, id builtinId) (
                // copy(x, y []T) int
                dst, _ := coreType(x.typ).(*Slice)
 
-               var y operand
-               arg(&y, 1)
-               if y.mode == invalid {
-                       return
-               }
+               y := args[1]
                src0 := coreString(y.typ)
                if src0 != nil && isString(src0) {
                        src0 = NewSlice(universeByte)
@@ -385,12 +363,12 @@ func (check *Checker) builtin(x *operand, call *syntax.CallExpr, id builtinId) (
                src, _ := src0.(*Slice)
 
                if dst == nil || src == nil {
-                       check.errorf(x, InvalidCopy, invalidArg+"copy expects slice arguments; found %s and %s", x, &y)
+                       check.errorf(x, InvalidCopy, invalidArg+"copy expects slice arguments; found %s and %s", x, y)
                        return
                }
 
                if !Identical(dst.elem, src.elem) {
-                       check.errorf(x, InvalidCopy, invalidArg+"arguments to copy %s and %s have different element types %s and %s", x, &y, dst.elem, src.elem)
+                       check.errorf(x, InvalidCopy, invalidArg+"arguments to copy %s and %s have different element types %s and %s", x, y, dst.elem, src.elem)
                        return
                }
 
@@ -422,11 +400,7 @@ func (check *Checker) builtin(x *operand, call *syntax.CallExpr, id builtinId) (
                        return
                }
 
-               arg(x, 1) // k
-               if x.mode == invalid {
-                       return
-               }
-
+               *x = *args[1] // key
                check.assignment(x, key, "argument to delete")
                if x.mode == invalid {
                        return
@@ -597,13 +571,10 @@ func (check *Checker) builtin(x *operand, call *syntax.CallExpr, id builtinId) (
                var params []Type
                if nargs > 0 {
                        params = make([]Type, nargs)
-                       for i := 0; i < nargs; i++ {
-                               if i > 0 {
-                                       arg(x, i) // first argument already evaluated
-                               }
+                       for i, a := range args {
+                               *x = *a
                                check.assignment(x, nil, "argument to "+predeclaredFuncs[id].name)
                                if x.mode == invalid {
-                                       // TODO(gri) "use" all arguments?
                                        return
                                }
                                params[i] = x.typ
@@ -634,9 +605,8 @@ func (check *Checker) builtin(x *operand, call *syntax.CallExpr, id builtinId) (
                        return
                }
 
-               var y operand
-               arg(&y, 1)
-               if !check.isValidIndex(&y, InvalidUnsafeAdd, "length", true) {
+               y := args[1]
+               if !check.isValidIndex(y, InvalidUnsafeAdd, "length", true) {
                        return
                }
 
@@ -770,9 +740,8 @@ func (check *Checker) builtin(x *operand, call *syntax.CallExpr, id builtinId) (
                        return
                }
 
-               var y operand
-               arg(&y, 1)
-               if !check.isValidIndex(&y, InvalidUnsafeSlice, "length", false) {
+               y := args[1]
+               if !check.isValidIndex(y, InvalidUnsafeSlice, "length", false) {
                        return
                }
 
@@ -811,9 +780,8 @@ func (check *Checker) builtin(x *operand, call *syntax.CallExpr, id builtinId) (
                        return
                }
 
-               var y operand
-               arg(&y, 1)
-               if !check.isValidIndex(&y, InvalidUnsafeString, "length", false) {
+               y := args[1]
+               if !check.isValidIndex(y, InvalidUnsafeString, "length", false) {
                        return
                }
 
index 150613eee3d39a205dd910b11175f6e79abbb251..63a59262dfc1a0adcaf7aa8900ce523468e26fa6 100644 (file)
@@ -40,24 +40,28 @@ func (check *Checker) builtin(x *operand, call *ast.CallExpr, id builtinId) (_ b
                check.hasCallOrRecv = false
        }
 
-       // determine actual arguments
-       var arg func(*operand, int) // TODO(gri) remove use of arg getter in favor of using xlist directly
-       nargs := len(call.Args)
+       // Evaluate arguments for built-ins that use ordinary (value) arguments.
+       // For built-ins with special argument handling (make, new, etc.),
+       // evaluation is done by the respective built-in code.
+       var args []*operand // not valid for _Make, _New, _Offsetof, _Trace
+       var nargs int
        switch id {
        default:
-               // make argument getter
-               xlist := check.exprList(call.Args)
-               arg = func(x *operand, i int) { *x = *xlist[i] }
-               nargs = len(xlist)
-               // evaluate first argument, if present
-               if nargs > 0 {
-                       arg(x, 0)
-                       if x.mode == invalid {
+               // check all arguments
+               args = check.exprList(call.Args)
+               nargs = len(args)
+               for _, a := range args {
+                       if a.mode == invalid {
                                return
                        }
                }
+               // first argument is always in x
+               if nargs > 0 {
+                       *x = *args[0]
+               }
        case _Make, _New, _Offsetof, _Trace:
                // arguments require special handling
+               nargs = len(call.Args)
        }
 
        // check argument count
@@ -99,26 +103,20 @@ func (check *Checker) builtin(x *operand, call *ast.CallExpr, id builtinId) (_ b
                        default:
                                cause = check.sprintf("have %s", x)
                        }
-                       // don't use Checker.invalidArg here as it would repeat "argument" in the error message
+                       // don't use invalidArg prefix here as it would repeat "argument" in the error message
                        check.errorf(x, InvalidAppend, "first argument to append must be a slice; %s", cause)
                        return
                }
 
-               // remember arguments that have been evaluated already
-               alist := []operand{*x}
-
                // spec: "As a special case, append also accepts a first argument assignable
                // to type []byte with a second argument of string type followed by ... .
                // This form appends the bytes of the string.
                if nargs == 2 && call.Ellipsis.IsValid() {
                        if ok, _ := x.assignableTo(check, NewSlice(universeByte), nil); ok {
-                               arg(x, 1)
-                               if x.mode == invalid {
-                                       return
-                               }
-                               if t := coreString(x.typ); t != nil && isString(t) {
+                               y := args[1]
+                               if t := coreString(y.typ); t != nil && isString(t) {
                                        if check.Types != nil {
-                                               sig := makeSig(S, S, x.typ)
+                                               sig := makeSig(S, S, y.typ)
                                                sig.variadic = true
                                                check.recordBuiltinType(call.Fun, sig)
                                        }
@@ -126,25 +124,13 @@ func (check *Checker) builtin(x *operand, call *ast.CallExpr, id builtinId) (_ b
                                        x.typ = S
                                        break
                                }
-                               alist = append(alist, *x)
-                               // fallthrough
                        }
                }
 
                // check general case by creating custom signature
                sig := makeSig(S, S, NewSlice(T)) // []T required for variadic signature
                sig.variadic = true
-               var alist2 []*operand
-               // convert []operand to []*operand
-               for i := range alist {
-                       alist2 = append(alist2, &alist[i])
-               }
-               for i := len(alist); i < nargs; i++ {
-                       var x operand
-                       arg(&x, i)
-                       alist2 = append(alist2, &x)
-               }
-               check.arguments(call, sig, nil, nil, alist2, nil, nil) // discard result (we know the result type)
+               check.arguments(call, sig, nil, nil, args, nil, nil) // discard result (we know the result type)
                // ok to continue even if check.arguments reported errors
 
                x.mode = value
@@ -278,11 +264,7 @@ func (check *Checker) builtin(x *operand, call *ast.CallExpr, id builtinId) (_ b
 
        case _Complex:
                // complex(x, y floatT) complexT
-               var y operand
-               arg(&y, 1)
-               if y.mode == invalid {
-                       return
-               }
+               y := args[1]
 
                // convert or check untyped arguments
                d := 0
@@ -300,7 +282,7 @@ func (check *Checker) builtin(x *operand, call *ast.CallExpr, id builtinId) (_ b
                        check.convertUntyped(x, y.typ)
                case 2:
                        // only y is untyped => convert to type of x
-                       check.convertUntyped(&y, x.typ)
+                       check.convertUntyped(y, x.typ)
                case 3:
                        // x and y are untyped =>
                        // 1) if both are constants, convert them to untyped
@@ -317,10 +299,10 @@ func (check *Checker) builtin(x *operand, call *ast.CallExpr, id builtinId) (_ b
                                        }
                                }
                                toFloat(x)
-                               toFloat(&y)
+                               toFloat(y)
                        } else {
                                check.convertUntyped(x, Typ[Float64])
-                               check.convertUntyped(&y, Typ[Float64])
+                               check.convertUntyped(y, Typ[Float64])
                                // x and y should be invalid now, but be conservative
                                // and check below
                        }
@@ -374,11 +356,7 @@ func (check *Checker) builtin(x *operand, call *ast.CallExpr, id builtinId) (_ b
                // copy(x, y []T) int
                dst, _ := coreType(x.typ).(*Slice)
 
-               var y operand
-               arg(&y, 1)
-               if y.mode == invalid {
-                       return
-               }
+               y := args[1]
                src0 := coreString(y.typ)
                if src0 != nil && isString(src0) {
                        src0 = NewSlice(universeByte)
@@ -386,12 +364,12 @@ func (check *Checker) builtin(x *operand, call *ast.CallExpr, id builtinId) (_ b
                src, _ := src0.(*Slice)
 
                if dst == nil || src == nil {
-                       check.errorf(x, InvalidCopy, invalidArg+"copy expects slice arguments; found %s and %s", x, &y)
+                       check.errorf(x, InvalidCopy, invalidArg+"copy expects slice arguments; found %s and %s", x, y)
                        return
                }
 
                if !Identical(dst.elem, src.elem) {
-                       check.errorf(x, InvalidCopy, "arguments to copy %s and %s have different element types %s and %s", x, &y, dst.elem, src.elem)
+                       check.errorf(x, InvalidCopy, "arguments to copy %s and %s have different element types %s and %s", x, y, dst.elem, src.elem)
                        return
                }
 
@@ -423,11 +401,7 @@ func (check *Checker) builtin(x *operand, call *ast.CallExpr, id builtinId) (_ b
                        return
                }
 
-               arg(x, 1) // k
-               if x.mode == invalid {
-                       return
-               }
-
+               *x = *args[1] // key
                check.assignment(x, key, "argument to delete")
                if x.mode == invalid {
                        return
@@ -598,13 +572,10 @@ func (check *Checker) builtin(x *operand, call *ast.CallExpr, id builtinId) (_ b
                var params []Type
                if nargs > 0 {
                        params = make([]Type, nargs)
-                       for i := 0; i < nargs; i++ {
-                               if i > 0 {
-                                       arg(x, i) // first argument already evaluated
-                               }
+                       for i, a := range args {
+                               *x = *a
                                check.assignment(x, nil, "argument to "+predeclaredFuncs[id].name)
                                if x.mode == invalid {
-                                       // TODO(gri) "use" all arguments?
                                        return
                                }
                                params[i] = x.typ
@@ -635,9 +606,8 @@ func (check *Checker) builtin(x *operand, call *ast.CallExpr, id builtinId) (_ b
                        return
                }
 
-               var y operand
-               arg(&y, 1)
-               if !check.isValidIndex(&y, InvalidUnsafeAdd, "length", true) {
+               y := args[1]
+               if !check.isValidIndex(y, InvalidUnsafeAdd, "length", true) {
                        return
                }
 
@@ -771,9 +741,8 @@ func (check *Checker) builtin(x *operand, call *ast.CallExpr, id builtinId) (_ b
                        return
                }
 
-               var y operand
-               arg(&y, 1)
-               if !check.isValidIndex(&y, InvalidUnsafeSlice, "length", false) {
+               y := args[1]
+               if !check.isValidIndex(y, InvalidUnsafeSlice, "length", false) {
                        return
                }
 
@@ -812,9 +781,8 @@ func (check *Checker) builtin(x *operand, call *ast.CallExpr, id builtinId) (_ b
                        return
                }
 
-               var y operand
-               arg(&y, 1)
-               if !check.isValidIndex(&y, InvalidUnsafeString, "length", false) {
+               y := args[1]
+               if !check.isValidIndex(y, InvalidUnsafeString, "length", false) {
                        return
                }