Clarify the += operator

[ndmitchell]

From the spec:

For most types, x += y is equivalent to x = x + y, except that it evaluates x only once, that is, it allocates a new list to hold the concatenation of x and y. However, if x refers to a list, the statement does not allocate a new list but instead mutates the original list in place, similar to x.extend(y).

Questions:

• What if the x is a frozen list? Is it then an error?
• When it says similar to, does it mean identically to? In particular, does it accept an iterator?

If so, is the correct translation of x += y:

if type(x) == "list": # but only for the built-in list type
    x.extend(y)
else:
    x = x + y # but only evaluate x once

[cjhopman]

Alternatively, is it like python's += (https://docs.python.org/3/reference/datamodel.html#object.iadd) where it's defined to first try using a type-specific in place add and falling back to normal addition if it's not supported?

My understanding is the intention is that it is like python. That matches how bazel and starlark-java treats it, and I'd argue is the most appropriate behavior. It seems particularly important for bazel so that user expectations are met when they do list += select.

starlark-go implements the fallback here: https://github.com/google/starlark-go/blob/7a1108eaa0124ea025e567d9feb4e41aab4bb024/starlark/interp.go#L200-L218

starlark-java implements the fallback here: https://github.com/bazelbuild/bazel/blob/master/src/main/java/net/starlark/java/eval/Eval.java#L454-L464

[laurentlb]

A frozen list should throw an error.

I don't think existing Starlark implementations allow new types to define their own += implementation, but there's no reason they shouldn't.

I think @adonovan wants to allow list += iterator (while list + iterator is forbidden). I don't have a strong preference here.

[adonovan]

@laurentlb I agree with all three points.

[cjhopman]

I think one of the things @ndmitchell wanted to clarify was that the spec explicitly says "if x refers to a list, the statement does not allocate a new list but instead mutates the original list in place, similar to x.extend(y)." That statement implies that you cannot do list += select because that can't possibly mutate the list in place and isn't like x.extend(y). The rust implementation is currently following that interpretation of the spec. Given the java and go implementations not following that behavior, I don't think that's the intention. If it is the intention, I think I'd want us to reconsider that as I think the python behavior and current starlark go and java behavior is better for users.

[adonovan]

I think the spec should be changed to say"if x refers to a list and y is also a list..." to reflect our current intention.

There remains the question of whether we should widen the special case to "list += iterable". (A Bazel select is not iterable.)

[ndmitchell]

So there seem to be two choices that need to be made for what x += y means:

• Everyone agrees: if x is a list and y is a list, this expression is equivalent to x.extend(y).
• Everyone agrees: if x is a frozen list, it raises an error.
• Choice: if y is an iterable, is it equivalent to x.extend(y)?
• Choice: if y isn't an iterable (or perhaps isn't a list depending on the previous answer) is equivalent to x = x + y or an error?

[adonovan]

My preferences:

Choice: if y is an iterable, is it equivalent to x.extend(y)?

Yes. It's useful, and consistent with the behaviors of both x.extend(y) in Starlark, and of x += y in Python.

Choice: if y isn't an iterable (or perhaps isn't a list depending on the previous answer) is equivalent to x = x + y or an error?

If y isn't iterable, x += y should be an error ("ytype is not iterable"). Again, this is consistent with extend in Starlark, and with the behavior of Python.

The spec wording and implementation work for both changes should be straightforward.

[ndmitchell]

@adonovan - while the spec and implementation work is straightforward, I wonder how many things this breaks. In particular, as @cjhopman notes, Bazel users who do:

my_list += select({a: b})

Currently get the behaviour my_list = my_list + select({a:b}), which I believe is translated down to my_list = select({a:my_list+b}). We'd suddenly be turning that into an error.

[adonovan]

You're absolutely right, I neglected the select case entirely. The fallback (as implemented by Go) seems like the right behavior, if the test of y for iterable (as also implemented the Go) fails.