Skip to content

Commit

Permalink
resolver: make -nesteddef and -lambda always on
Browse files Browse the repository at this point in the history
See bazelbuild/starlark#145
for spec changes.

Updates bazelbuild/starlark#20

Change-Id: I31e6258cc6caef6bcd3eab57ccec04f1b858b7e7
  • Loading branch information
adonovan committed Dec 4, 2020
1 parent 42d4f56 commit 152a019
Show file tree
Hide file tree
Showing 12 changed files with 26 additions and 66 deletions.
1 change: 0 additions & 1 deletion cmd/starlark/starlark.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ func init() {
flag.BoolVar(&resolve.AllowFloat, "float", resolve.AllowFloat, "obsolete; no effect")
flag.BoolVar(&resolve.AllowSet, "set", resolve.AllowSet, "allow set data type")
flag.BoolVar(&resolve.AllowLambda, "lambda", resolve.AllowLambda, "allow lambda expressions")
flag.BoolVar(&resolve.AllowNestedDef, "nesteddef", resolve.AllowNestedDef, "allow nested def statements")
flag.BoolVar(&resolve.AllowRecursion, "recursion", resolve.AllowRecursion, "allow while statements and recursive functions")
flag.BoolVar(&resolve.AllowGlobalReassign, "globalreassign", resolve.AllowGlobalReassign, "allow reassignment of globals, and if/for/while statements at top level")
}
Expand Down
21 changes: 2 additions & 19 deletions doc/spec.md
Original file line number Diff line number Diff line change
Expand Up @@ -2513,13 +2513,6 @@ def twice(x):
twice = lambda x: x * 2
```
<b>Implementation note:</b>
The Go implementation of Starlark requires the `-lambda` flag
to enable support for lambda expressions.
The Java implementation does not support them.
See Google Issue b/36358844.
## Statements
```grammar {.good}
Expand Down Expand Up @@ -2734,12 +2727,6 @@ current module.
<!-- this is too implementation-oriented; it's not a spec. -->
<b>Implementation note:</b>
The Go implementation of Starlark requires the `-nesteddef`
flag to enable support for nested `def` statements.
The Java implementation does not permit a `def` expression to be
nested within the body of another function.
### Return statements
Expand Down Expand Up @@ -4239,14 +4226,10 @@ applications to mimic the Bazel dialect more closely. Our goal is
eventually to eliminate all such differences on a case-by-case basis.
See [Starlark spec issue 20](https://github.com/bazelbuild/starlark/issues/20).
* Integers are represented with infinite precision.
* Integer arithmetic is exact.
* String interpolation supports the `[ioxXc]` conversions.
* `def` statements may be nested (option: `-nesteddef`).
* `lambda` expressions are supported (option: `-lambda`).
* String interpolation supports the `[ixXc]` conversions.
* String elements are bytes.
* Non-ASCII strings are encoded using UTF-8.
* Strings support octal and hex byte escapes.
* Strings support hex byte escapes.
* Strings have the additional methods `elem_ords`, `codepoint_ords`, and `codepoints`.
* The `chr` and `ord` built-in functions are supported.
* The `set` built-in function is provided (option: `-set`).
Expand Down
16 changes: 6 additions & 10 deletions resolve/resolve.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,14 +98,16 @@ const doesnt = "this Starlark dialect does not "
// These features are either not standard Starlark (yet), or deprecated
// features of the BUILD language, so we put them behind flags.
var (
AllowNestedDef = false // allow def statements within function bodies
AllowLambda = false // allow lambda expressions
AllowFloat = false // obsolete; no effect
AllowSet = false // allow the 'set' built-in
AllowGlobalReassign = false // allow reassignment to top-level names; also, allow if/for/while at top-level
AllowRecursion = false // allow while statements and recursive functions
AllowBitwise = true // obsolete; bitwise operations (&, |, ^, ~, <<, and >>) are always enabled
LoadBindsGlobally = false // load creates global not file-local bindings (deprecated)

// obsolete flags for features that are now standard. No effect.
AllowNestedDef = true
AllowLambda = true
AllowFloat = true
AllowBitwise = true
)

// File resolves the specified file and records information about the
Expand Down Expand Up @@ -506,9 +508,6 @@ func (r *resolver) stmt(stmt syntax.Stmt) {
r.assign(stmt.LHS, isAugmented)

case *syntax.DefStmt:
if !AllowNestedDef && r.container().function != nil {
r.errorf(stmt.Def, doesnt+"support nested def")
}
r.bind(stmt.Name)
fn := &Function{
Name: stmt.Name.Name,
Expand Down Expand Up @@ -780,9 +779,6 @@ func (r *resolver) expr(e syntax.Expr) {
}

case *syntax.LambdaExpr:
if !AllowLambda {
r.errorf(e.Lambda, doesnt+"support lambda")
}
fn := &Function{
Name: "lambda",
Pos: e.Lambda,
Expand Down
4 changes: 1 addition & 3 deletions resolve/resolve_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ import (

func setOptions(src string) {
resolve.AllowGlobalReassign = option(src, "globalreassign")
resolve.AllowLambda = option(src, "lambda")
resolve.AllowNestedDef = option(src, "nesteddef")
resolve.AllowRecursion = option(src, "recursion")
resolve.AllowSet = option(src, "set")
resolve.LoadBindsGlobally = option(src, "loadbindsglobally")
Expand All @@ -37,7 +35,7 @@ func TestResolve(t *testing.T) {
continue
}

// A chunk may set options by containing e.g. "option:nesteddef".
// A chunk may set options by containing e.g. "option:recursion".
setOptions(chunk.Source)

if err := resolve.File(f, isPredeclared, isUniversal); err != nil {
Expand Down
2 changes: 1 addition & 1 deletion resolve/testdata/resolve.star
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ _ = [x for x in "abc"]
M(x) ### "undefined: x"

---
# Functions may have forward refs. (option:lambda option:nesteddef)
# Functions may have forward refs.
def f():
g()
h() ### "undefined: h"
Expand Down
2 changes: 0 additions & 2 deletions starlark/eval_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@ import (
func setOptions(src string) {
resolve.AllowGlobalReassign = option(src, "globalreassign")
resolve.LoadBindsGlobally = option(src, "loadbindsglobally")
resolve.AllowLambda = option(src, "lambda")
resolve.AllowNestedDef = option(src, "nesteddef")
resolve.AllowRecursion = option(src, "recursion")
resolve.AllowSet = option(src, "set")
}
Expand Down
2 changes: 0 additions & 2 deletions starlark/testdata/assign.star
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,6 @@ assert.eq(m, 3)
assert.eq(n, 4)

---
# option:nesteddef
# misc assignment
load("assert.star", "assert")

Expand Down Expand Up @@ -176,7 +175,6 @@ f()
g = 1

---
# option:nesteddef
# Free variables are captured by reference, so this is ok.
load("assert.star", "assert")

Expand Down
1 change: 0 additions & 1 deletion starlark/testdata/benchmark.star
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
# Benchmarks of Starlark execution
# option:nesteddef

def bench_range_construction(b):
for _ in range(b.n):
Expand Down
1 change: 0 additions & 1 deletion starlark/testdata/dict.star
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
# Tests of Starlark 'dict'
# option:nesteddef

load("assert.star", "assert", "freeze")

Expand Down
5 changes: 2 additions & 3 deletions starlark/testdata/function.star
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# Tests of Starlark 'function'
# option:nesteddef option:set
# option:set

# TODO(adonovan):
# - add some introspection functions for looking at function values
Expand Down Expand Up @@ -239,7 +239,7 @@ assert.eq(y, ((1, 2, 4), dict(x=3, z=5)))
assert.eq(r, [1, 2, 3, 4, 5])

---
# option:nesteddef option:recursion
# option:recursion
# See github.com/bazelbuild/starlark#170
load("assert.star", "assert")

Expand Down Expand Up @@ -276,7 +276,6 @@ def e():
assert.eq(e(), 1)

---
# option:nesteddef
load("assert.star", "assert")

def e():
Expand Down
29 changes: 14 additions & 15 deletions starlark/testdata/list.star
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
# Tests of Starlark 'list'
# option:nesteddef

load("assert.star", "assert", "freeze")

Expand All @@ -16,14 +15,14 @@ assert.true(not [])

# indexing, x[i]
abc = list("abc".elems())
assert.fails(lambda : abc[-4], "list index -4 out of range \\[-3:2]")
assert.fails(lambda: abc[-4], "list index -4 out of range \\[-3:2]")
assert.eq(abc[-3], "a")
assert.eq(abc[-2], "b")
assert.eq(abc[-1], "c")
assert.eq(abc[0], "a")
assert.eq(abc[1], "b")
assert.eq(abc[2], "c")
assert.fails(lambda : abc[3], "list index 3 out of range \\[-3:2]")
assert.fails(lambda: abc[3], "list index 3 out of range \\[-3:2]")

# x[i] = ...
x3 = [0, 1, 2]
Expand All @@ -45,8 +44,8 @@ assert.fails(x3.clear, "cannot clear frozen list")

# list + list
assert.eq([1, 2, 3] + [3, 4, 5], [1, 2, 3, 3, 4, 5])
assert.fails(lambda : [1, 2] + (3, 4), "unknown.*list \\+ tuple")
assert.fails(lambda : (1, 2) + [3, 4], "unknown.*tuple \\+ list")
assert.fails(lambda: [1, 2] + (3, 4), "unknown.*list \\+ tuple")
assert.fails(lambda: (1, 2) + [3, 4], "unknown.*tuple \\+ list")

# list * int, int * list
assert.eq(abc * 0, [])
Expand All @@ -73,8 +72,8 @@ assert.eq([(y, x) for x, y in {1: 2, 3: 4}.items()], [(2, 1), (4, 3)])

# corner cases of parsing:
assert.eq([x for x in range(12) if x % 2 == 0 if x % 3 == 0], [0, 6])
assert.eq([x for x in [1, 2] if lambda : None], [1, 2])
assert.eq([x for x in [1, 2] if (lambda : 3 if True else 4)], [1, 2])
assert.eq([x for x in [1, 2] if lambda: None], [1, 2])
assert.eq([x for x in [1, 2] if (lambda: 3 if True else 4)], [1, 2])

# list function
assert.eq(list(), [])
Expand All @@ -98,8 +97,8 @@ listcompblock()

# list.pop
x4 = [1, 2, 3, 4, 5]
assert.fails(lambda : x4.pop(-6), "index -6 out of range \\[-5:4]")
assert.fails(lambda : x4.pop(6), "index 6 out of range \\[-5:4]")
assert.fails(lambda: x4.pop(-6), "index -6 out of range \\[-5:4]")
assert.fails(lambda: x4.pop(6), "index 6 out of range \\[-5:4]")
assert.eq(x4.pop(), 5)
assert.eq(x4, [1, 2, 3, 4])
assert.eq(x4.pop(1), 2)
Expand Down Expand Up @@ -205,12 +204,12 @@ def remove(v):
assert.eq(remove(3), [1, 4, 1])
assert.eq(remove(1), [3, 4, 1])
assert.eq(remove(4), [3, 1, 1])
assert.fails(lambda : [3, 1, 4, 1].remove(42), "remove: element not found")
assert.fails(lambda: [3, 1, 4, 1].remove(42), "remove: element not found")

# list.index
bananas = list("bananas".elems())
assert.eq(bananas.index("a"), 1) # bAnanas
assert.fails(lambda : bananas.index("d"), "value not in list")
assert.fails(lambda: bananas.index("d"), "value not in list")

# start
assert.eq(bananas.index("a", -1000), 1) # bAnanas
Expand All @@ -220,14 +219,14 @@ assert.eq(bananas.index("a", 2), 3) # banAnas
assert.eq(bananas.index("a", 3), 3) # banAnas
assert.eq(bananas.index("b", 0), 0) # Bananas
assert.eq(bananas.index("n", -3), 4) # banaNas
assert.fails(lambda : bananas.index("n", -2), "value not in list")
assert.fails(lambda: bananas.index("n", -2), "value not in list")
assert.eq(bananas.index("s", -2), 6) # bananaS
assert.fails(lambda : bananas.index("b", 1), "value not in list")
assert.fails(lambda: bananas.index("b", 1), "value not in list")

# start, end
assert.eq(bananas.index("s", -1000, 7), 6) # bananaS
assert.fails(lambda : bananas.index("s", -1000, 6), "value not in list")
assert.fails(lambda : bananas.index("d", -1000, 1000), "value not in list")
assert.fails(lambda: bananas.index("s", -1000, 6), "value not in list")
assert.fails(lambda: bananas.index("d", -1000, 1000), "value not in list")

# slicing, x[i:j:k]
assert.eq(bananas[6::-2], list("snnb".elems()))
Expand Down
8 changes: 0 additions & 8 deletions starlarkstruct/struct_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,11 @@ import (
"path/filepath"
"testing"

"go.starlark.net/resolve"
"go.starlark.net/starlark"
"go.starlark.net/starlarkstruct"
"go.starlark.net/starlarktest"
)

func init() {
// The tests make extensive use of these not-yet-standard features.
resolve.AllowLambda = true
resolve.AllowNestedDef = true
resolve.AllowSet = true
}

func Test(t *testing.T) {
testdata := starlarktest.DataFile("starlarkstruct", ".")
thread := &starlark.Thread{Load: load}
Expand Down

0 comments on commit 152a019

Please sign in to comment.