Specify whether **kwargs is frozen

[brandjon]

Python lets you mutate **kwargs inside the function, but not *args since it's a tuple.

The Java implementation currently lets you mutate **kwargs, though a recent change at head removes this ability for certain types of calls. We should fix that for consistency, but also specify it here in the spec.

cc @tetromino

[tetromino]

Note that we cannot deeply freeze **kwargs because it may contain mutable values which the caller may mutate further post-call.

[laurentlb]

• Dictionaries are normally mutable or deeply frozen. I don't think we have any other case at the moment (so it could be weird to add a special case here).

• I remember seeing examples where people intentionally mutated kwargs, e.g. to add/remove values before passing down kwargs to another macro in bazel.

I think it makes sense to have kwargs mutable, and it doesn't seem worth making a breaking change here (unless someone finds a better rationale?)

[tetromino]

(Weak) rationale for thinking about this:

• it's a bit strange that*args is immutable (by virtue of being a tuple) and **kwargs isn't;
• GC churn from very frequently creating new empty mutable dicts to pass as **kwargs (but need to measure to see whether this at all matters)

[stepancheg]

(Weak) rationale for thinking about this:

• Opportunity to do optimization:

def foo(**kwargs): pass

def bar(**kwargs): foo(**kwargs)

If kwargs is immutable, we can avoid copying.

• Similarly:

def foo(**kwargs): pass

def bar(): foo(x = 1, y = 2)

We could allocate kwargs statically (when optimizer knows the signature of foo at the time of compilation of bar).

[brandjon]

Note that we cannot deeply freeze **kwargs

Right, it would be shallow only, like how tuples are always at least shallowly immutable.

Dictionaries are normally mutable or deeply frozen. I don't think we have any other case at the moment (so it could be weird to add a special case here).

Yes, if we did it we'd have to make sure that we don't confuse a shallowly immutable dict with a deeply frozen one. (Which makes me realize that our constructors in the Java code for creating frozen dicts/lists might be misused with mutable elements / entries. Or at least, the javadoc doesn't caution against that strongly enough.)

I remember seeing examples where people intentionally mutated kwargs, e.g. to add/remove values before passing down kwargs to another macro in bazel.

Now that you mention this, this is super common and I've done it myself plenty of times in my limited macro-writing experience. So I think this is a show-stopper for immutable kwargs.

it's a bit strange that *args is immutable (by virtue of being a tuple) and **kwargs isn't;

Yes, but it's Python's wart rather than ours. Then again, Python doesn't have a concept of pervasive freezing, and if we were doing it all over from scratch it wouldn't necessarily be that strange to say that **kwargs's value is a frozendict.

I'm not too moved by the optimization / GC argument, since I think that's secondary to use cases like macro authors mutating kwargs in place.

[brandjon]

Discussed offline and we agreed that kwargs should remain mutable.

It looks only a very small percentage of .bzl files match a regex looking for assignments to kwargs keys, but the number is still high enough that it'd be an annoying migration relative to the gain.

Also, the hypothetical GC improvement only applies to functions that declare a **kwargs parameter, which is probably mainly macros, i.e. functions that define one or more targets. So we think this minor optimization is easily amortized over the unavoidable work being done in those functions.

This issue can stay open until the spec is clarified.