Repository: google/starlark-go
Pull request #510 (2023-09-22 11:28:29)

Make resolve follow FileOptions.TopLevelControl
The new struct `FileOptions` allows for fine grained control over language feature.

However, it didn't respect `FileOptions.TopLevelControl` flag, as the only way to actually allow if/for/while in the top level context was to enable `FileOptions.GlobalReassign` flag.



Patch:
diff --git a/resolve/resolve.go b/resolve/resolve.go
index 8c780d8..c576a6b 100644
--- a/resolve/resolve.go
+++ b/resolve/resolve.go
@@ -505,7 +505,7 @@ func (r *resolver) stmt(stmt syntax.Stmt) {
 		}
 
 	case *syntax.IfStmt:
-		if !r.options.GlobalReassign && r.container().function == nil {
+		if !r.options.TopLevelControl && r.container().function == nil {
 			r.errorf(stmt.If, "if statement not within a function")
 		}
 		r.expr(stmt.Cond)
@@ -531,7 +531,7 @@ func (r *resolver) stmt(stmt syntax.Stmt) {
 		r.function(fn, stmt.Def)
 
 	case *syntax.ForStmt:
-		if !r.options.GlobalReassign && r.container().function == nil {
+		if !r.options.TopLevelControl && r.container().function == nil {
 			r.errorf(stmt.For, "for loop not within a function")
 		}
 		r.expr(stmt.X)
@@ -545,7 +545,7 @@ func (r *resolver) stmt(stmt syntax.Stmt) {
 		if !r.options.While {
 			r.errorf(stmt.While, doesnt+"support while loops")
 		}
-		if !r.options.GlobalReassign && r.container().function == nil {
+		if !r.options.TopLevelControl && r.container().function == nil {
 			r.errorf(stmt.While, "while loop not within a function")
 		}
 		r.expr(stmt.Cond)


Test patch:
diff --git a/resolve/resolve_test.go b/resolve/resolve_test.go
index 95a9445..31bba18 100644
--- a/resolve/resolve_test.go
+++ b/resolve/resolve_test.go
@@ -16,17 +16,13 @@ import (
 
 // A test may enable non-standard options by containing (e.g.) "option:recursion".
 func getOptions(src string) *syntax.FileOptions {
-	// TODO(adonovan): use new fine-grained names.
-	// And share with eval_test.go
-	allowGlobalReassign := option(src, "globalreassign")
-	recursion := option(src, "recursion")
 	return &syntax.FileOptions{
 		Set:               option(src, "set"),
-		While:             allowGlobalReassign || recursion,
-		TopLevelControl:   allowGlobalReassign,
-		GlobalReassign:    allowGlobalReassign,
+		While:             option(src, "while"),
+		TopLevelControl:   option(src, "toplevelcontrol"),
+		GlobalReassign:    option(src, "globalreassign"),
 		LoadBindsGlobally: option(src, "loadbindsglobally"),
-		Recursion:         recursion,
+		Recursion:         option(src, "recursion"),
 	}
 }
 
diff --git a/resolve/testdata/resolve.star b/resolve/testdata/resolve.star
index 4fca831..cb6385a 100644
--- a/resolve/testdata/resolve.star
+++ b/resolve/testdata/resolve.star
@@ -143,17 +143,17 @@ load("foo",
      _e="f") # ok
 
 ---
-# option:globalreassign
+# option:toplevelcontrol
 if M:
     load("foo", "bar") ### "load statement within a conditional"
 
 ---
-# option:globalreassign
+# option:toplevelcontrol
 for x in M:
     load("foo", "bar") ### "load statement within a loop"
 
 ---
-# option:recursion option:globalreassign
+# option:toplevelcontrol option:while
 while M:
     load("foo", "bar") ### "load statement within a loop"
 
@@ -173,7 +173,7 @@ if x: ### "if statement not within a function"
   pass
 
 ---
-# option:globalreassign
+# option:toplevelcontrol
 
 for x in "abc": # ok
   pass
@@ -189,7 +189,7 @@ def f():
     pass
 
 ---
-# option:recursion
+# option:while
 
 def f():
   while U: # ok
@@ -199,7 +199,7 @@ while U: ### "while loop not within a function"
   pass
 
 ---
-# option:globalreassign option:recursion
+# option:toplevelcontrol option:while
 
 while U: # ok
   pass
diff --git a/starlark/eval_test.go b/starlark/eval_test.go
index dbab3ca..6678671 100644
--- a/starlark/eval_test.go
+++ b/starlark/eval_test.go
@@ -32,17 +32,13 @@ import (
 
 // A test may enable non-standard options by containing (e.g.) "option:recursion".
 func getOptions(src string) *syntax.FileOptions {
-	// TODO(adonovan): use new fine-grained names.
-	// And share with resolve_test.go
-	allowGlobalReassign := option(src, "globalreassign")
-	recursion := option(src, "recursion")
 	return &syntax.FileOptions{
 		Set:               option(src, "set"),
-		While:             allowGlobalReassign || recursion,
-		TopLevelControl:   allowGlobalReassign,
-		GlobalReassign:    allowGlobalReassign,
+		While:             option(src, "while"),
+		TopLevelControl:   option(src, "toplevelcontrol"),
+		GlobalReassign:    option(src, "globalreassign"),
 		LoadBindsGlobally: option(src, "loadbindsglobally"),
-		Recursion:         recursion,
+		Recursion:         option(src, "recursion"),
 	}
 }
 
@@ -141,6 +137,7 @@ func TestExecFile(t *testing.T) {
 		"testdata/tuple.star",
 		"testdata/recursion.star",
 		"testdata/module.star",
+		"testdata/while.star",
 	} {
 		filename := filepath.Join(testdata, file)
 		for _, chunk := range chunkedfile.Read(filename, t) {
diff --git a/starlark/testdata/recursion.star b/starlark/testdata/recursion.star
index 3368614..7029ea0 100644
--- a/starlark/testdata/recursion.star
+++ b/starlark/testdata/recursion.star
@@ -6,38 +6,9 @@
 
 load("assert.star", "assert")
 
-def sum(n):
-	r = 0
-	while n > 0:
-		r += n
-		n -= 1
-	return r
-
 def fib(n):
 	if n <= 1:
 		return 1
 	return fib(n-1) + fib(n-2)
 
-def while_break(n):
-	r = 0
-	while n > 0:
-		if n == 5:
-			break
-		r += n
-		n -= 1
-	return r
-
-def while_continue(n):
-	r = 0
-	while n > 0:
-		if n % 2 == 0:
-			n -= 1
-			continue
-		r += n
-		n -= 1
-	return r
-
 assert.eq(fib(5), 8)
-assert.eq(sum(5), 5+4+3+2+1)
-assert.eq(while_break(10), 40)
-assert.eq(while_continue(10), 25)
diff --git a/starlark/testdata/while.star b/starlark/testdata/while.star
new file mode 100644
index 0000000..676a3b4
--- /dev/null
+++ b/starlark/testdata/while.star
@@ -0,0 +1,37 @@
+# Tests of Starlark while statement.
+
+# This is a "chunked" file: each "---" effectively starts a new file.
+
+# option:while
+
+load("assert.star", "assert")
+
+def sum(n):
+	r = 0
+	while n > 0:
+		r += n
+		n -= 1
+	return r
+
+def while_break(n):
+	r = 0
+	while n > 0:
+		if n == 5:
+			break
+		r += n
+		n -= 1
+	return r
+
+def while_continue(n):
+	r = 0
+	while n > 0:
+		if n % 2 == 0:
+			n -= 1
+			continue
+		r += n
+		n -= 1
+	return r
+
+assert.eq(sum(5), 5+4+3+2+1)
+assert.eq(while_break(10), 40)
+assert.eq(while_continue(10), 25)