From 74cff922fe29145fe93dab2b06ac81bf83ef30e3 Mon Sep 17 00:00:00 2001
From: alandonovan <adonovan@google.com>
Date: Fri, 4 Dec 2020 17:09:48 -0500
Subject: [PATCH] resolver: make -nesteddef and -lambda always on

See https://github.com/bazelbuild/starlark/pull/145
for spec changes.

Updates https://github.com/bazelbuild/starlark/issues/20

Change-Id: I31e6258cc6caef6bcd3eab57ccec04f1b858b7e7
---
 cmd/starlark/starlark.go         |  1 -
 doc/spec.md                      | 17 -----------------
 resolve/resolve.go               | 16 ++++++----------
 resolve/resolve_test.go          |  4 +---
 resolve/testdata/resolve.star    |  2 +-
 starlark/eval_test.go            |  2 --
 starlark/testdata/assign.star    |  2 --
 starlark/testdata/benchmark.star |  1 -
 starlark/testdata/dict.star      |  1 -
 starlark/testdata/function.star  |  5 ++---
 starlark/testdata/list.star      | 29 ++++++++++++++---------------
 starlarkstruct/struct_test.go    |  8 --------
 12 files changed, 24 insertions(+), 64 deletions(-)

diff --git a/cmd/starlark/starlark.go b/cmd/starlark/starlark.go
index 31555f1e..3825f003 100644
--- a/cmd/starlark/starlark.go
+++ b/cmd/starlark/starlark.go
@@ -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")
 }
diff --git a/doc/spec.md b/doc/spec.md
index 04602857..92b1ab94 100644
--- a/doc/spec.md
+++ b/doc/spec.md
@@ -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}
@@ -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
 
@@ -4239,11 +4226,7 @@ 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 elements are bytes.
 * Non-ASCII strings are encoded using UTF-8.
 * Strings support octal and hex byte escapes.
diff --git a/resolve/resolve.go b/resolve/resolve.go
index 8c1b7304..56e33ba5 100644
--- a/resolve/resolve.go
+++ b/resolve/resolve.go
@@ -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
@@ -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,
@@ -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,
diff --git a/resolve/resolve_test.go b/resolve/resolve_test.go
index cb2c6724..50d1cc50 100644
--- a/resolve/resolve_test.go
+++ b/resolve/resolve_test.go
@@ -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")
@@ -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 {
diff --git a/resolve/testdata/resolve.star b/resolve/testdata/resolve.star
index fb4c78e0..ce671105 100644
--- a/resolve/testdata/resolve.star
+++ b/resolve/testdata/resolve.star
@@ -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"
diff --git a/starlark/eval_test.go b/starlark/eval_test.go
index 3725d17c..a95ab2ea 100644
--- a/starlark/eval_test.go
+++ b/starlark/eval_test.go
@@ -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")
 }
diff --git a/starlark/testdata/assign.star b/starlark/testdata/assign.star
index 64d60fb6..7f579f0b 100644
--- a/starlark/testdata/assign.star
+++ b/starlark/testdata/assign.star
@@ -90,7 +90,6 @@ assert.eq(m, 3)
 assert.eq(n, 4)
 
 ---
-# option:nesteddef
 # misc assignment
 load("assert.star", "assert")
 
@@ -176,7 +175,6 @@ f()
 g = 1
 
 ---
-# option:nesteddef
 # Free variables are captured by reference, so this is ok.
 load("assert.star", "assert")
 
diff --git a/starlark/testdata/benchmark.star b/starlark/testdata/benchmark.star
index 85d6d944..b02868d9 100644
--- a/starlark/testdata/benchmark.star
+++ b/starlark/testdata/benchmark.star
@@ -1,5 +1,4 @@
 # Benchmarks of Starlark execution
-# option:nesteddef
 
 def bench_range_construction(b):
     for _ in range(b.n):
diff --git a/starlark/testdata/dict.star b/starlark/testdata/dict.star
index 9864be73..1aeb1e7c 100644
--- a/starlark/testdata/dict.star
+++ b/starlark/testdata/dict.star
@@ -1,5 +1,4 @@
 # Tests of Starlark 'dict'
-# option:nesteddef
 
 load("assert.star", "assert", "freeze")
 
diff --git a/starlark/testdata/function.star b/starlark/testdata/function.star
index 9b74740a..bdfa5acc 100644
--- a/starlark/testdata/function.star
+++ b/starlark/testdata/function.star
@@ -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
@@ -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")
 
@@ -276,7 +276,6 @@ def e():
 assert.eq(e(), 1)
 
 ---
-# option:nesteddef
 load("assert.star", "assert")
 
 def e():
diff --git a/starlark/testdata/list.star b/starlark/testdata/list.star
index ff98b968..526a9626 100644
--- a/starlark/testdata/list.star
+++ b/starlark/testdata/list.star
@@ -1,5 +1,4 @@
 # Tests of Starlark 'list'
-# option:nesteddef
 
 load("assert.star", "assert", "freeze")
 
@@ -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]
@@ -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, [])
@@ -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(), [])
@@ -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)
@@ -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
@@ -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()))
diff --git a/starlarkstruct/struct_test.go b/starlarkstruct/struct_test.go
index 5de7020e..4f103bdf 100644
--- a/starlarkstruct/struct_test.go
+++ b/starlarkstruct/struct_test.go
@@ -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}