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

resolve: a non-binding use of a global may precede its binding #123

Merged
merged 1 commit into from
Aug 22, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions doc/spec.md
Original file line number Diff line number Diff line change
Expand Up @@ -1140,6 +1140,7 @@ nested in any order, to any depth.
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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why continue to maintain this document instead of pointing to https://docs.bazel.build/versions/master/skylark/spec.html? There should be one true spec.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This document accurately describes the language implemented by Skylark-in-Go. A single true spec would be nice, but for the Bazel team, support for uses of Skylark other than Bazel does not appear to be a priority right now, so Skylark-in-Java lags behind some features that we agreed upon such as support for the kinds of numbers encountered in protocol buffers, separation of x.f() calls into two steps y = x.f; y(), the string methods elem_ords, codepoints, and codepoint_ords, and so on.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My recommendation is to point to https://github.com/bazelbuild/starlark/blob/master/spec.md
and document only the differences. Hopefully we can reduce the list of differences over time.

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
Expand Down
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
13 changes: 8 additions & 5 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"

---
# 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()
Expand Down Expand Up @@ -79,7 +85,6 @@ def f():
i()
i = lambda: 0


def g():
f()

Expand Down Expand Up @@ -115,8 +120,6 @@ e = 1

---
# 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():
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