Skip to content

Commit

Permalink
universe: remove freeze(x) built-in function (google#42)
Browse files Browse the repository at this point in the history
It's still useful for testing though, so we move it to assert.sky.
  • Loading branch information
adonovan authored Nov 10, 2017
1 parent 4b42bbf commit ffb61b8
Show file tree
Hide file tree
Showing 14 changed files with 30 additions and 53 deletions.
1 change: 0 additions & 1 deletion cmd/skylark/skylark.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ var (
// non-standard dialect flags
func init() {
flag.BoolVar(&resolve.AllowFloat, "fp", resolve.AllowFloat, "allow floating-point numbers")
flag.BoolVar(&resolve.AllowFreeze, "freeze", resolve.AllowFreeze, "add freeze built-in function")
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")
Expand Down
8 changes: 4 additions & 4 deletions doc/impl.md
Original file line number Diff line number Diff line change
Expand Up @@ -147,10 +147,10 @@ simple bit flip, no need to traverse the object graph---but coarser
grained. Also, it complicates the API slightly because to construct a
list, say, requires a reference to the frozen flag it should use.

The Go implementation also permits the `freeze` built-in to be exposed
to the program. (This requires the `-freeze` dialect flag.) This has
proven valuable in writing tests of the freeze mechanism itself, but
is mostly a curiosity.
The Go implementation would also permit the freeze operation to be
exposed to the program, for example as a built-in function.
This has proven valuable in writing tests of the freeze mechanism
itself, but is otherwise mostly a curiosity.


### Fail-fast iterators
Expand Down
28 changes: 4 additions & 24 deletions doc/spec.md
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,6 @@ concurrency, and other such features of Python.
* [dir](#dir)
* [enumerate](#enumerate)
* [float](#float)
* [freeze](#freeze)
* [getattr](#getattr)
* [hasattr](#hasattr)
* [hash](#hash)
Expand Down Expand Up @@ -916,14 +915,17 @@ If the function becomes frozen, its parameters' default values become
frozen too.

```python
# module a.sky
def f(x, list=[]):
list.append(x)
return list

f(4, [1,2,3]) # [1, 2, 3, 4]
f(1) # [1]
f(2) # [1, 2], not [2]!
freeze(f)

# module b.sky
load("a.sky", "f")
f(3) # error: cannot append to frozen list
```

Expand Down Expand Up @@ -1235,7 +1237,6 @@ application-defined, implement a few basic behaviors:
```text
str(x) -- return a string representation of x
type(x) -- return a string describing the type of x
freeze(x) -- make x, and everything it transitively refers to, immutable
bool(x) -- convert x to a Boolean truth value
hash(x) -- return a hash code for x
```
Expand Down Expand Up @@ -1311,13 +1312,6 @@ and .bzl files in parallel, and two modules being executed
concurrently may freely access variables or call functions from a
third without the possibility of a race condition.

<b>Implementation note:</b>
The Go implementation of Skylark permits user code to freeze arbitrary
values by calling the `freeze` built-in function.
This feature must be enabled in the REPL by the `-freeze` flag.
This function is not present in the Java implementation, which freezes
values only _en masse_ at the end of module initialization.

### Hashing

The `dict` and `set` data types are implemented using hash tables, so
Expand Down Expand Up @@ -2850,19 +2844,6 @@ function, and the real division operator `/`.
The Java implementation does not yet support floating-point numbers.


### freeze

`freeze(x)` freezes x and all values transitively reachable from it.
Subsequent attempts to modify any of those values will fail.

At the end of module execution, the value of each global in the module
dictionary is frozen as if by `freeze`.

<b>Implementation note:</b>
The `freeze` function is an optional feature of the Go implementation,
and it must be enabled in the REPL using the `-freeze` flag.
It is not present in the Java implementation.

### getattr

`getattr(x, name)` returns the value of the attribute (field or method) of x named `name`.
Expand Down Expand Up @@ -3922,7 +3903,6 @@ eventually to eliminate all such differences on a case-by-case basis.
* `assert` is a valid identifier.
* `&` is a token; `int & int` and `set & set` are supported.
* `int | int` is supported.
* The `freeze` built-in function is provided (option: `-freeze`).
* The parser accepts unary `+` expressions.
* A method call `x.f()` may be separated into two steps: `y = x.f; y()`.
* Dot expressions may appear on the left side of an assignment: `x.f = 1`.
Expand Down
1 change: 0 additions & 1 deletion eval_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ func init() {
resolve.AllowLambda = true
resolve.AllowNestedDef = true
resolve.AllowFloat = true
resolve.AllowFreeze = true
resolve.AllowSet = true
}

Expand Down
14 changes: 1 addition & 13 deletions library.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,7 @@ func init() {
"dict": NewBuiltin("dict", dict),
"dir": NewBuiltin("dir", dir),
"enumerate": NewBuiltin("enumerate", enumerate),
"float": NewBuiltin("float", float), // requires resolve.AllowFloat
"freeze": NewBuiltin("freeze", freeze), // requires resolve.AllowFreeze
"float": NewBuiltin("float", float), // requires resolve.AllowFloat
"getattr": NewBuiltin("getattr", getattr),
"hasattr": NewBuiltin("hasattr", hasattr),
"hash": NewBuiltin("hash", hash),
Expand Down Expand Up @@ -506,17 +505,6 @@ func float(thread *Thread, _ *Builtin, args Tuple, kwargs []Tuple) (Value, error
}
}

func freeze(thread *Thread, _ *Builtin, args Tuple, kwargs []Tuple) (Value, error) {
if len(kwargs) > 0 {
return nil, fmt.Errorf("freeze does not accept keyword arguments")
}
if len(args) != 1 {
return nil, fmt.Errorf("freeze got %d arguments, wants 1", len(args))
}
args[0].Freeze()
return args[0], nil
}

// https://github.com/google/skylark/blob/master/doc/spec.md#getattr
func getattr(thread *Thread, _ *Builtin, args Tuple, kwargs []Tuple) (Value, error) {
var object, dflt Value
Expand Down
4 changes: 0 additions & 4 deletions resolve/resolve.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@ var (
AllowNestedDef = false // allow def statements within function bodies
AllowLambda = false // allow lambda expressions
AllowFloat = false // allow floating point literals, the 'float' built-in, and x / y
AllowFreeze = false // allow the 'freeze' built-in
AllowSet = false // allow the 'set' built-in
AllowGlobalReassign = false // allow reassignment to globals declared in same file (deprecated)
)
Expand Down Expand Up @@ -346,9 +345,6 @@ func (r *resolver) useGlobal(id *syntax.Ident) (scope Scope) {
if !AllowSet && id.Name == "set" {
r.errorf(id.NamePos, doesnt+"support sets")
}
if !AllowFreeze && id.Name == "freeze" {
r.errorf(id.NamePos, doesnt+"provide the 'freeze' built-in function")
}
} else {
scope = Undefined
r.errorf(id.NamePos, "undefined: %s", id.Name)
Expand Down
1 change: 0 additions & 1 deletion resolve/resolve_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ func TestResolve(t *testing.T) {
resolve.AllowNestedDef = option(chunk.Source, "nesteddef")
resolve.AllowLambda = option(chunk.Source, "lambda")
resolve.AllowFloat = option(chunk.Source, "float")
resolve.AllowFreeze = option(chunk.Source, "freeze")
resolve.AllowSet = option(chunk.Source, "set")
resolve.AllowGlobalReassign = option(chunk.Source, "global_reassign")

Expand Down
Binary file added skylark
Binary file not shown.
3 changes: 3 additions & 0 deletions skylarktest/assert.sky
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
# catch(f): evaluate f() and returns its evaluation error message, if any
# matches(str, pattern): report whether str matches regular expression pattern.
# struct: a constructor for a simple HasFields implementation.
# freeze(x): freeze the value x and everything reachable from it.
#
# Clients may use these functions to define their own testing abstractions.

Expand Down Expand Up @@ -36,6 +37,8 @@ def _fails(f, pattern):
elif not matches(pattern, msg):
error("regular expression (%s) did not match error (%s)" % (pattern, msg))

freeze = freeze # an exported global whose value is the built-in freeze function

assert = struct(
fail = error,
eq = _eq,
Expand Down
13 changes: 13 additions & 0 deletions skylarktest/skylarktest.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ func LoadAssertModule() (skylark.StringDict, error) {
"catch": skylark.NewBuiltin("catch", catch),
"matches": skylark.NewBuiltin("matches", matches),
"struct": skylark.NewBuiltin("struct", skylarkstruct.Make),
"freeze": skylark.NewBuiltin("freeze", freeze),
}
filename := DataFile("skylark/skylarktest", "assert.sky")
thread := new(skylark.Thread)
Expand Down Expand Up @@ -116,6 +117,18 @@ func error_(thread *skylark.Thread, _ *skylark.Builtin, args skylark.Tuple, kwar
return skylark.None, nil
}

// freeze(x) freezes its operand.
func freeze(thread *skylark.Thread, _ *skylark.Builtin, args skylark.Tuple, kwargs []skylark.Tuple) (skylark.Value, error) {
if len(kwargs) > 0 {
return nil, fmt.Errorf("freeze does not accept keyword arguments")
}
if len(args) != 1 {
return nil, fmt.Errorf("freeze got %d arguments, wants 1", len(args))
}
args[0].Freeze()
return args[0], nil
}

// DataFile returns the effective filename of the specified
// test data resource. The function abstracts differences between
// 'go build', under which a test runs in its package directory,
Expand Down
2 changes: 1 addition & 1 deletion testdata/assign.sky
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ assert.eq(z[0], 1)

---
# assignment to/from fields.
load("assert.sky", "assert")
load("assert.sky", "assert", "freeze")

hf = hasfields()
hf.x = 1
Expand Down
2 changes: 1 addition & 1 deletion testdata/dict.sky
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Tests of Skylark 'dict'

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

# literals
assert.eq({}, {})
Expand Down
4 changes: 2 additions & 2 deletions testdata/function.sky
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
# and test that functions have correct position, free vars, names of locals, etc.
# - move the hard-coded tests of parameter passing from eval_test.go to here.

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

# Test lexical scope and closures:
def outer(x):
Expand Down Expand Up @@ -104,7 +104,7 @@ assert.eq(len(hashes), 1)

---
# Default values of function parameters are mutable.
load("assert.sky", "assert")
load("assert.sky", "assert", "freeze")

def f(x=[0]):
return x
Expand Down
2 changes: 1 addition & 1 deletion testdata/list.sky
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Tests of Skylark 'list'

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

# literals
assert.eq([], [])
Expand Down

0 comments on commit ffb61b8

Please sign in to comment.