Bazel: incompatible_disallow_dict_plus: disallow dictionary concatenation

[laurentlb]

We are removing the + operator on dictionaries. This includes the += form where the left-hand side is a dictionary. This is done to improve compatibility with Python. A possible workaround is to use the .update method instead.

Flag: --incompatible_disallow_dict_plus

[kastiglione]

How are python compatibility goals weighed? Having this + means Starlark code using it wouldn't be directly runnable by a python interpreter, but is that even a goal? The reason I ask is because + is a convenient and more declarative syntax for use in build files.

[brandjon]

Dave, did you have any specific examples in mind where + is much more readable, compared with calling a utility function?

Our only hard constraint is that we make Starlark syntactically a subset of Python 2 and 3 from the parser's point of view. That's not an issue in this case. But one of our guiding principles is that we like Starlark's semantics to resemble a subset of Python's, even though there may be differences in the details. A core data type supporting an extra built-in operation is a semantic difference.

The fact that some Starlark code runs in a Python interpreter is not a design goal, but a temporary technical constraint. The reason I'm opposed to + on dicts is more for compatibility with the Python programmer rather than with the Python interpreter.

[kastiglione]

Dave, did you have any specific examples in mind where + is much more readable

Not handy, but I will try to follow up another time with something to consider.

The reason I'm opposed to + on dicts is more for compatibility with the Python programmer rather than with the Python interpreter.

Agreed. I mentioned "build files" in my original comment. Within .bzl files, I have no comment here, not having + is reasonable. The flip side is BUILD files where many people will not know Python and just want a convenient syntax. Such folks wouldn't know or care. I realize you can't have it available in one and not the other. But this issue did make me curious about the goals/motivations.

[philwo]

This flag was not flipped in time for the Bazel 0.23.0 release and will thus be postponed to Bazel 0.24.0.

[AustinSchuh]

@brandjon and crew, the use case we have is that we build up starlark fragments to build up rules. For example:

_common_attrs = { 
  'srcs': attr.label_list(allow_files=_srcs_files,
      mandatory=False, non_empty=False),
  'deps': attr.label_list(allow_files=False, providers=custom_cc_providers),
  'hdrs': attr.label_list(allow_files=_header_files),
  'textual_hdrs': attr.label_list(allow_files=True),
  'copts': attr.string_list(mandatory=False),
  'linkopts': attr.string_list(mandatory=False),
  'includes': attr.string_list(mandatory=False),
  'alwayslink': attr.bool(default=False),
  'licenses': attr.license(),
  # This is the path (not including the trailing /) of the repository if the
  # repository is an external repository.  There is no mechanism to get this
  # from bazel that Austin or Brian can find yet.
  'repository_prefix': attr.string(default=''),
}

common_binary_attrs = _common_attrs + {'stamp': attr.bool(default=False)}

_custom_cc_binary_raw = rule(
    custom_cc_binary,
    attrs = common_binary_attrs + _target_attrs +
            _target_attrs_for_stdlib + _target_coverage_attr,
    executable = True,
    outputs = _binary_outputs,
)

This lets us not duplicate the attributes pretty cleanly. Is there a better pattern we should be using? update won't work here.

Side note, I've seen a proposal to add back a + operator for dictionaries for this exact purpose. Of course, I can't find it now...

[vladmos]

The recommended workardound is to use dicts.add from Skylib: https://github.com/bazelbuild/bazel-skylib/blob/master/docs/dicts_doc.md

[laurentlb]

If you want to avoid the dependency on skylib, you could also do:

common_binary_attrs = dict(_common_attrs)
common_binary_attrs.update(stamp = attr.bool(default=False))

The argument of dict.update can be a dictionary or just keyword arguments.

[kastiglione]

As an alternative to update, this example can be done with the dict() constructor alone, because it takes keyword args in addition to a dict:

python common_binary_attrs = dict( _common_attrs, stamp = attr.bool(default=False) )

[vladmos]

Yet another workaround: if you want to avoid the dependency on Skylib and don't want to create an intermediate variable to add several dicts, you can use the following:

rule (
    attrs = dict(d1.items() + d2.items() + d3.items())
)

[kastiglione]

EDIT: this is the same as what @vladmos wrote.

Another option, is adding dictionary .items() and wrapping the result with the dict() constructor:

attrs = dict((
    common_binary_attrs.items() +
    _target_attrs.items() +
    _target_attrs_for_stdlib.items() +
    _target_coverage_attr.items()
))