Skip to content

Commit

Permalink
resolve: if AllowGlobalReassign (google#14)
Browse files Browse the repository at this point in the history
This change causes the AllowGlobalReassign flag (used for legacy Bazel
semantics) to also allow use of predeclared names prior to a global
binding of the same name, as in:

   print(len); len=1; print(len)

This effectively reverses github.com/google/skylark/pull/123 behind an option.

See github.com/google/skylark/issues/116 for discussion.
  • Loading branch information
adonovan authored Nov 26, 2018
1 parent 3632e37 commit 84a935b
Show file tree
Hide file tree
Showing 2 changed files with 60 additions and 0 deletions.
35 changes: 35 additions & 0 deletions resolve/resolve.go
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,41 @@ func (r *resolver) bind(id *syntax.Ident, allowRebind bool) bool {
}

func (r *resolver) use(id *syntax.Ident) {
// The spec says that if there is a global binding of a name
// then all references to that name in that block refer to the
// global, even if the use precedes the def---just as for locals.
// For example, in this code,
//
// print(len); len=1; print(len)
//
// both occurrences of len refer to the len=1 binding, which
// completely shadows the predeclared len function.
//
// The rationale for these semantics, which differ from Python,
// is that the static meaning of len (a reference to a global)
// does not change depending on where it appears in the file.
// Of course, its dynamic meaning does change, from an error
// into a valid reference, so it's not clear these semantics
// have any practical advantage.
//
// In any case, the Bazel implementation lags behind the spec
// and follows Python behavior, so the first use of len refers
// to the predeclared function. This typically used in a BUILD
// file that redefines a predeclared name half way through,
// for example:
//
// proto_library(...) # built-in rule
// load("myproto.bzl", "proto_library")
// proto_library(...) # user-defined rule
//
// We will piggyback support for the legacy semantics on the
// AllowGlobalReassign flag, which is loosely related and also
// required for Bazel.
if AllowGlobalReassign && r.env.isModule() {
r.useGlobal(id)
return
}

b := r.container()
b.uses = append(b.uses, use{id, r.env})
}
Expand Down
25 changes: 25 additions & 0 deletions resolve/testdata/resolve.star
Original file line number Diff line number Diff line change
Expand Up @@ -245,3 +245,28 @@ c = 3.141 ### `dialect does not support floating point`
a = float("3.141")
b = 1 / 2
c = 3.141

---
# option:global_reassign
# Legacy Bazel (and Python) semantics: def must precede use even for globals.

_ = x ### `undefined: x`
x = 1

---
# option:global_reassign
# Legacy Bazel (and Python) semantics: reassignment of globals is allowed.
x = 1
x = 2 # ok

---
# option:global_reassign
# Redeclaration of predeclared names is allowed.

# module-specific predeclared name
M = 1 # ok
M = 2 # ok (legacy)

# universal predeclared name
U = 1 # ok
U = 1 # ok (legacy)

0 comments on commit 84a935b

Please sign in to comment.