]> Cypherpunks repositories - gostls13.git/commitdiff
go/types: delay type-checking of function literals
authorRobert Griesemer <gri@golang.org>
Mon, 11 Dec 2017 23:30:39 +0000 (15:30 -0800)
committerRobert Griesemer <gri@golang.org>
Mon, 12 Feb 2018 21:40:02 +0000 (21:40 +0000)
R=go1.11

Functions (at the package level) were collected and their bodies
type-checked after all other package-level objects were checked.
But function literals where type-checked right away when they were
encountered so that they could see the correct, partially populated
surrounding scope, and also to mark variables of the surrounding
function as used.

This approach, while simple, breaks down in esoteric cases where
a function literal appears inside the declaration of an object
that its body depends on: If the body is type-checked before the
object is completely set up, the literal may use incomplete data
structures, possibly leading to spurious errors.

This change postpones type-checking of function literals to later;
after the current expression or statement, but before any changes
to the enclosing scope (so that the delayed type-checking sees the
correct scope contents).

The new mechanism is general and now is also used for other
(non-function) delayed checks.

Fixes #22992.

Change-Id: Ic95f709560858b4bdf8c645be70abe4449f6184d
Reviewed-on: https://go-review.googlesource.com/83397
Reviewed-by: Alan Donovan <adonovan@google.com>
src/go/types/check.go
src/go/types/decl.go
src/go/types/expr.go
src/go/types/resolver.go
src/go/types/stmt.go
src/go/types/testdata/constdecl.src
src/go/types/testdata/cycles.src
src/go/types/testdata/cycles5.src
src/go/types/testdata/init0.src
src/go/types/testdata/vardecl.src
src/go/types/typexpr.go

index b046458cf7de12f9e4a300991c0ef480e38f552a..af2ce9e605c8063500902b3dfe6ff31eba30ae3e 100644 (file)
@@ -39,14 +39,6 @@ type exprInfo struct {
        val   constant.Value // constant value; or nil (if not a constant)
 }
 
-// funcInfo stores the information required for type-checking a function.
-type funcInfo struct {
-       name string    // for debugging/tracing only
-       decl *declInfo // for cycle detection
-       sig  *Signature
-       body *ast.BlockStmt
-}
-
 // A context represents the context within which an object is type-checked.
 type context struct {
        decl          *declInfo      // package-level declaration whose init expression/function body is checked
@@ -96,8 +88,7 @@ type Checker struct {
        methods    map[string][]*Func       // maps package scope type names to associated non-blank, non-interface methods
        interfaces map[*TypeName]*ifaceInfo // maps interface type names to corresponding interface infos
        untyped    map[ast.Expr]exprInfo    // map of expressions without final type
-       funcs      []funcInfo               // list of functions to type-check
-       delayed    []func()                 // delayed checks requiring fully setup types
+       delayed    []func()                 // stack of delayed actions
 
        // context within which the current object is type-checked
        // (valid only for the duration of type-checking a specific object)
@@ -153,11 +144,11 @@ func (check *Checker) rememberUntyped(e ast.Expr, lhs bool, mode operandMode, ty
        m[e] = exprInfo{lhs, mode, typ, val}
 }
 
-func (check *Checker) later(name string, decl *declInfo, sig *Signature, body *ast.BlockStmt) {
-       check.funcs = append(check.funcs, funcInfo{name, decl, sig, body})
-}
-
-func (check *Checker) delay(f func()) {
+// later pushes f on to the stack of actions that will be processed later;
+// either at the end of the current statement, or in case of a local constant
+// or variable declaration, before the constant or variable is in scope
+// (so that f still sees the scope before any new declarations).
+func (check *Checker) later(f func()) {
        check.delayed = append(check.delayed, f)
 }
 
@@ -195,7 +186,6 @@ func (check *Checker) initFiles(files []*ast.File) {
        check.methods = nil
        check.interfaces = nil
        check.untyped = nil
-       check.funcs = nil
        check.delayed = nil
 
        // determine package name and collect valid files
@@ -246,17 +236,10 @@ func (check *Checker) checkFiles(files []*ast.File) (err error) {
 
        check.packageObjects()
 
-       check.functionBodies()
+       check.processDelayed(0) // incl. all functions
 
        check.initOrder()
 
-       // perform delayed checks
-       // (cannot use range - delayed checks may add more delayed checks;
-       // e.g., when type-checking delayed embedded interfaces)
-       for i := 0; i < len(check.delayed); i++ {
-               check.delayed[i]()
-       }
-
        if !check.conf.DisableUnusedImportCheck {
                check.unusedImports()
        }
index f78aed9368762dfc4afad49b5cb1bb6b5ea69251..8faa2e1f7ef7f92dec58b560f4e2d99009964a00 100644 (file)
@@ -355,7 +355,9 @@ func (check *Checker) funcDecl(obj *Func, decl *declInfo) {
        // function body must be type-checked after global declarations
        // (functions implemented elsewhere have no body)
        if !check.conf.IgnoreFuncBodies && fdecl.Body != nil {
-               check.later(obj.name, decl, sig, fdecl.Body)
+               check.later(func() {
+                       check.funcBody(decl, obj.name, sig, fdecl.Body)
+               })
        }
 }
 
@@ -373,6 +375,8 @@ func (check *Checker) declStmt(decl ast.Decl) {
                        case *ast.ValueSpec:
                                switch d.Tok {
                                case token.CONST:
+                                       top := len(check.delayed)
+
                                        // determine which init exprs to use
                                        switch {
                                        case s.Type != nil || len(s.Values) > 0:
@@ -397,6 +401,9 @@ func (check *Checker) declStmt(decl ast.Decl) {
 
                                        check.arityMatch(s, last)
 
+                                       // process function literals in init expressions before scope changes
+                                       check.processDelayed(top)
+
                                        // spec: "The scope of a constant or variable identifier declared
                                        // inside a function begins at the end of the ConstSpec or VarSpec
                                        // (ShortVarDecl for short variable declarations) and ends at the
@@ -407,6 +414,8 @@ func (check *Checker) declStmt(decl ast.Decl) {
                                        }
 
                                case token.VAR:
+                                       top := len(check.delayed)
+
                                        lhs0 := make([]*Var, len(s.Names))
                                        for i, name := range s.Names {
                                                lhs0[i] = NewVar(name.Pos(), pkg, name.Name, nil)
@@ -447,6 +456,9 @@ func (check *Checker) declStmt(decl ast.Decl) {
 
                                        check.arityMatch(s, nil)
 
+                                       // process function literals in init expressions before scope changes
+                                       check.processDelayed(top)
+
                                        // declare all variables
                                        // (only at this point are the variable scopes (parents) set)
                                        scopePos := s.End() // see constant declarations
index 59534c75708b959e60a2449c4c457f3d68a8846d..aec35b61c83e6806d23e36c45614fceb50d95406 100644 (file)
@@ -1030,16 +1030,14 @@ func (check *Checker) exprInternal(x *operand, e ast.Expr, hint Type) exprKind {
                        // Anonymous functions are considered part of the
                        // init expression/func declaration which contains
                        // them: use existing package-level declaration info.
-                       //
-                       // TODO(gri) We delay type-checking of regular (top-level)
-                       //           function bodies until later. Why don't we do
-                       //           it for closures of top-level expressions?
-                       //           (We can't easily do it for local closures
-                       //           because the surrounding scopes must reflect
-                       //           the exact position where the closure appears
-                       //           in the source; e.g., variables declared below
-                       //           must not be visible).
-                       check.funcBody(check.decl, "", sig, e.Body)
+                       decl := check.decl // capture for use in closure below
+                       // Don't type-check right away because the function may
+                       // be part of a type definition to which the function
+                       // body refers. Instead, type-check as soon as possible,
+                       // but before the enclosing scope contents changes (#22992).
+                       check.later(func() {
+                               check.funcBody(decl, "<function literal>", sig, e.Body)
+                       })
                        x.mode = value
                        x.typ = sig
                } else {
index b5bec13d9d31aa2f765f454d6693fc92b2a145a7..a49bf4961ddf165a6f073a3965292bbeb4fbca5a 100644 (file)
@@ -493,10 +493,13 @@ func (a inSourceOrder) Len() int           { return len(a) }
 func (a inSourceOrder) Less(i, j int) bool { return a[i].order() < a[j].order() }
 func (a inSourceOrder) Swap(i, j int)      { a[i], a[j] = a[j], a[i] }
 
-// functionBodies typechecks all function bodies.
-func (check *Checker) functionBodies() {
-       for _, f := range check.funcs {
-               check.funcBody(f.decl, f.name, f.sig, f.body)
+// processDelayed processes all delayed actions pushed after top.
+func (check *Checker) processDelayed(top int) {
+       for len(check.delayed) > top {
+               i := len(check.delayed) - 1
+               f := check.delayed[i]
+               check.delayed = check.delayed[:i]
+               f() // may append to check.delayed
        }
 }
 
index af43c804a80426ce2883f2e8ff0a40e5f97e0c49..23b395c87c26b5340ff29a8a2cb131ee0bf37d2e 100644 (file)
@@ -7,7 +7,6 @@
 package types
 
 import (
-       "fmt"
        "go/ast"
        "go/constant"
        "go/token"
@@ -16,11 +15,10 @@ import (
 
 func (check *Checker) funcBody(decl *declInfo, name string, sig *Signature, body *ast.BlockStmt) {
        if trace {
-               if name == "" {
-                       name = "<function literal>"
-               }
-               fmt.Printf("--- %s: %s {\n", name, sig)
-               defer fmt.Println("--- <end>")
+               check.trace(body.Pos(), "--- %s: %s", name, sig)
+               defer func() {
+                       check.trace(body.End(), "--- <end>")
+               }()
        }
 
        // set function scope extent
@@ -52,8 +50,6 @@ func (check *Checker) funcBody(decl *declInfo, name string, sig *Signature, body
 
        // spec: "Implementation restriction: A compiler may make it illegal to
        // declare a variable inside a function body if the variable is never used."
-       // (One could check each scope after use, but that distributes this check
-       // over several places because CloseScope is not always called explicitly.)
        check.usage(sig.scope)
 }
 
@@ -72,7 +68,7 @@ func (check *Checker) usage(scope *Scope) {
        }
 
        for _, scope := range scope.children {
-               // Don't go inside closure scopes a second time;
+               // Don't go inside function literal scopes a second time;
                // they are handled explicitly by funcBody.
                if !scope.isFunc {
                        check.usage(scope)
@@ -309,6 +305,9 @@ func (check *Checker) stmt(ctxt stmtContext, s ast.Stmt) {
                }(check.scope)
        }
 
+       // process collected function literals before scope changes
+       defer check.processDelayed(len(check.delayed))
+
        inner := ctxt &^ (fallthroughOk | finalSwitchCase)
        switch s := s.(type) {
        case *ast.BadStmt, *ast.EmptyStmt:
index 6de9b13d6ef3209d0f7ab71d5e6ee2328cec029b..c2f40ed6e6711a26474339e2e96de9e861c65c27 100644 (file)
@@ -5,6 +5,7 @@
 package constdecl
 
 import "math"
+import "unsafe"
 
 var v int
 
@@ -94,4 +95,16 @@ func _() {
        )
 }
 
+// Test case for constants depending on function literals (see also #22992).
+const A /* ERROR initialization cycle */ = unsafe.Sizeof(func() { _ = A })
+
+func _() {
+       // The function literal below must not see a.
+       const a = unsafe.Sizeof(func() { _ = a /* ERROR "undeclared name" */ })
+       const b = unsafe.Sizeof(func() { _ = a })
+
+       // The function literal below must not see x, y, or z.
+       const x, y, z = 0, 1, unsafe.Sizeof(func() { _ = x /* ERROR "undeclared name" */ + y /* ERROR "undeclared name" */ + z /* ERROR "undeclared name" */ })
+}
+
 // TODO(gri) move extra tests from testdata/const0.src into here
index 0165baa21a3920ab6fb8fd96ac36f715ad996239..79e75e9316d7f393933581a08d2e4c29eaf15bd3 100644 (file)
@@ -4,6 +4,8 @@
 
 package cycles
 
+import "unsafe"
+
 type (
        T0 int
        T1 /* ERROR cycle */ T1
@@ -150,3 +152,12 @@ type (
        T16 map[[len(T16 /* ERROR cycle */ {1:2})]int]int
        T17 map[int][len(T17 /* ERROR cycle */ {1:2})]int
 )
+
+// Test case for types depending on function literals (see also #22992).
+type T20 chan [unsafe.Sizeof(func(ch T20){ _ = <-ch })]byte
+type T22 = chan [unsafe.Sizeof(func(ch T20){ _ = <-ch })]byte
+
+func _() {
+       type T1 chan [unsafe.Sizeof(func(ch T1){ _ = <-ch })]byte
+       type T2 = chan [unsafe.Sizeof(func(ch T2){ _ = <-ch })]byte
+}
index 984da681bdaca5cc2a09b4af84b897a0b3aab0f4..aab9ee235ebfb153df9c9632e0f293bc69295dcd 100644 (file)
@@ -112,6 +112,8 @@ type (
 
 // arbitrary code may appear inside an interface
 
+const n = unsafe.Sizeof(func(){})
+
 type I interface {
-       m([unsafe.Sizeof(func() { I.m(nil) })]byte) // should report an error (see #22992)
+       m([unsafe.Sizeof(func() { I.m(nil, [n]byte{}) })]byte)
 }
index ef0349c70f5ff053f71fbceaae659093d8610a22..6e8746afb64197af29c089f8f9a368ca1a7b0186 100644 (file)
@@ -75,7 +75,7 @@ var x8 = x7
 func f2() (int, int) { return f3() + f3(), 0 }
 func f3() int { return x8 }
 
-// cycles via closures
+// cycles via function literals
 
 var x9 /* ERROR initialization cycle */ = func() int { return x9 }()
 
index 197dec2d5dc6900270b1820b05cd3ec472e5bc1e..54f5ef1e10d0983f55f65ea32f421e2b4e53ee63 100644 (file)
@@ -151,7 +151,7 @@ func (r T) _(a, b, c int) (u, v, w int) {
        return
 }
 
-// Unused variables in closures must lead to only one error (issue #22524).
+// Unused variables in function literals must lead to only one error (issue #22524).
 func _() {
        _ = func() {
                var x /* ERROR declared but not used */ int
@@ -190,4 +190,17 @@ func _() {
        _ = b
 }
 
+// Test case for variables depending on function literals (see also #22992).
+var A /* ERROR initialization cycle */ = func() int { return A }()
+
+func _() {
+       // The function literal below must not see a.
+       var a = func() int { return a /* ERROR "undeclared name" */ }()
+       var _ = func() int { return a }()
+
+       // The function literal below must not see x, y, or z.
+       var x, y, z = 0, 1, func() int { return x /* ERROR "undeclared name" */ + y /* ERROR "undeclared name" */ + z /* ERROR "undeclared name" */ }()
+       _, _, _ = x, y, z
+}
+
 // TODO(gri) consolidate other var decl checks in this file
\ No newline at end of file
index 883e62e0ba8e4c599c48c4b6353924fc20ea3aab..aedd71e9186e4d498bbab90427b9510630500325 100644 (file)
@@ -317,7 +317,7 @@ func (check *Checker) typExprInternal(e ast.Expr, def *Named, path []*TypeName)
                //
                // Delay this check because it requires fully setup types;
                // it is safe to continue in any case (was issue 6667).
-               check.delay(func() {
+               check.later(func() {
                        if !Comparable(typ.key) {
                                check.errorf(e.Key.Pos(), "invalid map key type %s", typ.key)
                        }
@@ -478,8 +478,8 @@ func (check *Checker) interfaceType(ityp *Interface, iface *ast.InterfaceType, d
        // collect embedded interfaces
        // Only needed for printing and API. Delay collection
        // to end of type-checking when all types are complete.
-       interfaceScope := check.scope // capture for use in delayed function
-       check.delay(func() {
+       interfaceScope := check.scope // capture for use in closure below
+       check.later(func() {
                check.scope = interfaceScope
                if trace {
                        check.trace(iface.Pos(), "-- delayed checking embedded interfaces of %s", iface)