resolve: a non-binding use of a global may precede its binding
According to the agreed Skylark spec, and unlike the Python semantics
currently implemented, this program should yield a dynamic error:
print(len) # dynamic error: uninitialized global
len = 1
print(len)
We now defer the resolution of forward-uses at top level until the end
of the module, just as we do for locals.
Fixes #116
Change-Id: I802bded5d50c45702a4da08c40144574cdf4296a
diff --git a/doc/spec.md b/doc/spec.md
index 051e07c..b7a611e 100644
--- a/doc/spec.md
+++ b/doc/spec.md
@@ -1140,6 +1140,7 @@
If name is bound anywhere within a block, all uses of the name within
the block are treated as references to that binding, even uses that
appear before the binding.
+This is true even in the module block, unlike Python.
The binding of `y` on the last line of the example below makes `y`
local to the function `hello`, so the use of `y` in the print
statement also refers to the local `y`, even though it appears
diff --git a/resolve/resolve.go b/resolve/resolve.go
index 0843bad..4c1e8f9 100644
--- a/resolve/resolve.go
+++ b/resolve/resolve.go
@@ -30,8 +30,11 @@
//
// Python-style resolution requires multiple passes because a name is
// determined to be local to a function only if the function contains a
-// "binding" use of it, and this use may lexically follow a non-binding
-// use. In the first pass, we inspect each function, recording in
+// "binding" use of it; similarly, a name is determined be global (as
+// opposed to predeclared) if the module contains a binding use at the
+// top level. For both locals and globals, a non-binding use may
+// lexically precede the binding to which it is resolved.
+// In the first pass, we inspect each function, recording in
// 'uses' each identifier and the environment block in which it occurs.
// If a use of a name is binding, such as a function parameter or
// assignment, we add the name to the block's bindings mapping and add a
@@ -66,7 +69,8 @@
//
// Skylark enforces that all global names are assigned at most once on
// all control flow paths by forbidding if/else statements and loops at
-// top level.
+// top level. A global may be used before it is defined, leading to a
+// dynamic error.
//
// TODO(adonovan): opt: reuse local slots once locals go out of scope.
@@ -332,12 +336,6 @@
}
func (r *resolver) use(id *syntax.Ident) {
- // Reference outside any local (comprehension/function) block?
- if r.env.isModule() {
- r.useGlobal(id)
- return
- }
-
b := r.container()
b.uses = append(b.uses, use{id, r.env})
}
diff --git a/resolve/testdata/resolve.sky b/resolve/testdata/resolve.sky
index 6ddcf20..bbad8e0 100644
--- a/resolve/testdata/resolve.sky
+++ b/resolve/testdata/resolve.sky
@@ -9,8 +9,8 @@
_ = x
---
-# premature use of global
-_ = x ### "undefined: x"
+# premature use of global is not a static error; see issue 116.
+_ = x
x = 1
---
@@ -37,6 +37,12 @@
U = 1 ### "cannot reassign global U declared at .*/resolve.sky"
---
+# A global declaration shadows all references to a predeclared; see issue 116.
+
+a = U # ok: U is a reference to the global defined on the next line.
+U = 1
+
+---
# reference to predeclared name
M()
@@ -79,7 +85,6 @@
i()
i = lambda: 0
-
def g():
f()
@@ -115,8 +120,6 @@
---
# This program should resolve successfully but fail dynamically.
-# However, the Java implementation currently reports the dynamic
-# error at the x=2 statement.
x = 1
def f():
diff --git a/testdata/assign.sky b/testdata/assign.sky
index ca6b3c5..fd87073 100644
--- a/testdata/assign.sky
+++ b/testdata/assign.sky
@@ -191,22 +191,26 @@
z += 3 ### "global variable z referenced before assignment"
---
-# It's ok to define a global that shadows a built-in.
-
load("assert.sky", "assert")
-assert.eq(type(list), "builtin_function_or_method")
+# It's ok to define a global that shadows a built-in...
list = []
assert.eq(type(list), "list")
-# set and float are dialect-specific,
-# but we shouldn't notice any difference.
+# ...but then all uses refer to the global,
+# even if they occur before the binding use.
+# See github.com/google/skylark/issues/116.
+assert.fails(lambda: tuple, "global variable tuple referenced before assignment")
+tuple = ()
-assert.eq(type(float), "builtin_function_or_method")
+---
+# Same as above, but set and float are dialect-specific;
+# we shouldn't notice any difference.
+load("assert.sky", "assert")
+
float = 1.0
assert.eq(type(float), "float")
-assert.eq(type(set), "builtin_function_or_method")
set = [1, 2, 3]
assert.eq(type(set), "list")
