resolve: allow if/for/while statements at toplevel

These constructs are still disallowed in the Bazel build language,
but it is easy to check for them as a separate pass over the syntax tree,
so there is no need to complicate the spec with this restriction,
which is a nuisance in other dialects.

Clients that set the -globalreassign flag to suppress the check
need do so no longer.

Change-Id: I39ddb2128e6151a878a884fdb84d7017df2d5d51

doc/spec.md

@@ -97,6 +97,7 @@ concurrency, and other such features of Python.
     * [Expression statements](#expression-statements)
     * [If statements](#if-statements)
     * [For loops](#for-loops)
+    * [While loops](#while-loops)
     * [Break and Continue](#break-and-continue)
     * [Load statements](#load-statements)
     * [Module execution](#module-execution)
@@ -2629,14 +2630,6 @@ else:
         result = 0
 ```
 
-An `if` statement is permitted only within a function definition.
-An `if` statement at top level results in a static error.
-
-<b>Implementation note:</b>
-The Go implementation of Starlark permits `if`-statements to appear at top-level
-if the `-globalreassign` flag is enabled.
-
-
 ### While loops
 
 A `while` loop evaluates an expression (the _condition_) and if the truth
@@ -2698,13 +2691,6 @@ Within the body of a `for` loop, `break` and `continue` statements may
 be used to stop the execution of the loop or advance to the next
 iteration.
 
-In Starlark, a `for` loop is permitted only within a function definition.
-A `for` loop at top level results in a static error.
-
-<b>Implementation note:</b>
-The Go implementation of Starlark permits loops to appear at top-level
-if the `-globalreassign` flag is enabled.
-
 
 ### Break and Continue
 
@@ -2776,7 +2762,8 @@ load("module.star", "x", "y", "z")       # assigns x, y, and z
 load("module.star", "x", y2="y", "z")    # assigns x, y2, and z
 ```
 
-A load statement within a function is a static error.
+Load statements must be at top level.
+It is a static error if a load statement appears within a function, if-statement, or a loop.
 
 
 ## Module execution
@@ -4034,5 +4021,4 @@ See [Starlark spec issue 20](https://github.com/bazelbuild/starlark/issues/20).
 * `hash` accepts operands besides strings.
 * `sorted` accepts the additional parameters `key` and `reverse`.
 * `type(x)` returns `"builtin_function_or_method"` for built-in functions.
-* `if`, `for`, and `while` are permitted at toplevel (option: `-globalreassign`).
 * top-level rebindings are permitted (option: `-globalreassign`).

resolve/resolve.go

@@ -93,7 +93,7 @@ var (
 	AllowLambda         = false // allow lambda expressions
 	AllowFloat          = false // allow floating point literals, the 'float' built-in, and x / y
 	AllowSet            = false // allow the 'set' built-in
-	AllowGlobalReassign = false // allow reassignment to globals declared in same file (deprecated)
+	AllowGlobalReassign = false // allow reassignment to globals declared in same file
 	AllowBitwise        = false // allow bitwise operations (&, |, ^, ~, <<, and >>)
 	AllowRecursion      = false // allow while statements and recursive functions
 )
@@ -211,6 +211,7 @@ type resolver struct {
 	isPredeclared, isUniversal func(name string) bool
 
 	loops int // number of enclosing for loops
+	ifs   int // number of enclosing if-statements
 
 	errors ErrorList
 }
@@ -431,12 +432,11 @@ func (r *resolver) stmt(stmt syntax.Stmt) {
 		}
 
 	case *syntax.IfStmt:
-		if !AllowGlobalReassign && r.container().function == nil {
-			r.errorf(stmt.If, "if statement not within a function")
-		}
 		r.expr(stmt.Cond)
+		r.ifs++
 		r.stmts(stmt.True)
 		r.stmts(stmt.False)
+		r.ifs--
 
 	case *syntax.AssignStmt:
 		if !AllowBitwise {
@@ -463,9 +463,6 @@ func (r *resolver) stmt(stmt syntax.Stmt) {
 		r.function(stmt.Def, stmt.Name.Name, &stmt.Function)
 
 	case *syntax.ForStmt:
-		if !AllowGlobalReassign && r.container().function == nil {
-			r.errorf(stmt.For, "for loop not within a function")
-		}
 		r.expr(stmt.X)
 		const allowRebind = false
 		r.assign(stmt.Vars, allowRebind)
@@ -477,9 +474,6 @@ func (r *resolver) stmt(stmt syntax.Stmt) {
 		if !AllowRecursion {
 			r.errorf(stmt.While, doesnt+"support while loops")
 		}
-		if !AllowGlobalReassign && r.container().function == nil {
-			r.errorf(stmt.While, "while loop not within a function")
-		}
 		r.expr(stmt.Cond)
 		r.loops++
 		r.stmts(stmt.Body)
@@ -494,8 +488,8 @@ func (r *resolver) stmt(stmt syntax.Stmt) {
 		}
 
 	case *syntax.LoadStmt:
-		if r.container().function != nil {
-			r.errorf(stmt.Load, "load statement within a function")
+		if r.ifs > 0 || r.loops > 0 || r.container().function != nil {
+			r.errorf(stmt.Load, "load statement not at top level")
 		}
 
 		const allowRebind = false

resolve/testdata/resolve.star

@@ -134,7 +134,7 @@ f()
 load("module", "name") # ok
 
 def f():
-  load("foo", "bar") ### "load statement within a function"
+  load("foo", "bar") ### "load statement not at top level"
 
 load("foo",
      "",     ### "load: empty identifier"
@@ -149,17 +149,8 @@ load("foo",
 return ### "return statement not within a function"
 
 ---
-# if-statements and for-loops at top-level are forbidden
-# (without globalreassign option)
-
-for x in "abc": ### "for loop not within a function"
-  pass
-
-if x: ### "if statement not within a function"
-  pass
-
----
-# option:globalreassign
+# if-statements and for-loops at top-level are OK in core Starlark,
+# but they are rejected by the Bazel BUILD/.bzl dialect.
 
 for x in "abc": # ok
   pass
@@ -170,22 +161,11 @@ if x: # ok
 ---
 # while loops are forbidden (without -recursion option)
 
-def f():
-  while U: ### "dialect does not support while loops"
-    pass
-
----
-# option:recursion
-
-def f():
-  while U: # ok
-    pass
-
-while U: ### "while loop not within a function"
+while U: ### "dialect does not support while loops"
   pass
 
 ---
-# option:globalreassign option:recursion
+# option:recursion
 
 while U: # ok
   pass