Skip to content
This repository has been archived by the owner on Dec 29, 2022. It is now read-only.

Commit

Permalink
resolve: a non-binding use of a global may precede its binding
Browse files Browse the repository at this point in the history
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: I6b9c12dda1b9c244748ab4f5a9acf4a43721c215
  • Loading branch information
adonovan committed Aug 22, 2018
1 parent 5b0e788 commit dc3c692
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 19 deletions.
16 changes: 7 additions & 9 deletions resolve/resolve.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,11 @@ package resolve
//
// 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
Expand Down Expand Up @@ -66,7 +69,8 @@ package resolve
//
// 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.

Expand Down Expand Up @@ -332,12 +336,6 @@ func (r *resolver) bind(id *syntax.Ident, allowRebind bool) bool {
}

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})
}
Expand Down
11 changes: 8 additions & 3 deletions resolve/testdata/resolve.sky
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ x = 1
_ = x

---
# premature use of global
_ = x ### "undefined: x"
# premature use of global is not a static error; see issue 116.
_ = x
x = 1

---
Expand All @@ -36,6 +36,12 @@ M = 2 ### "cannot reassign global M declared at .*/resolve.sky"
U = 1 # ok
U = 1 ### "cannot reassign global U declared at .*/resolve.sky"

---
# Regression test for github.com/google/skylark/issues/116

a = U # ok: U is a reference to the global defined on the next line.
U = 1

---
# reference to predeclared name
M()
Expand Down Expand Up @@ -79,7 +85,6 @@ def f():
i()
i = lambda: 0


def g():
f()

Expand Down
18 changes: 11 additions & 7 deletions testdata/assign.sky
Original file line number Diff line number Diff line change
Expand Up @@ -191,22 +191,26 @@ f()
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 = ()

---
# Same as above, but set and float are dialect-specific;
# we shouldn't notice any difference.
load("assert.sky", "assert")

assert.eq(type(float), "builtin_function_or_method")
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")

Expand Down

0 comments on commit dc3c692

Please sign in to comment.