-
Notifications
You must be signed in to change notification settings - Fork 4.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
incompatible_disallow_dict_plus: disallow dictionary concatenation #6461
Comments
How are python compatibility goals weighed? Having this |
Dave, did you have any specific examples in mind where 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 |
Not handy, but I will try to follow up another time with something to consider.
Agreed. I mentioned "build files" in my original comment. Within .bzl files, I have no comment here, not having |
This flag was not flipped in time for the Bazel 0.23.0 release and will thus be postponed to Bazel 0.24.0. |
The + operator on dictionaries will be removed See bazelbuild/bazel#6461 PiperOrigin-RevId: 236656561
See bazelbuild/bazel#6461 PiperOrigin-RevId: 238458969
@brandjon and crew, the use case we have is that we build up starlark fragments to build up rules. For example:
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... |
The recommended workardound is to use |
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. |
As an alternative to common_binary_attrs = dict(
_common_attrs,
stamp = attr.bool(default=False)
) |
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:
|
EDIT: this is the same as what @vladmos wrote. Another option, is adding dictionary attrs = dict((
common_binary_attrs.items() +
_target_attrs.items() +
_target_attrs_for_stdlib.items() +
_target_coverage_attr.items()
)) |
#6461 RELNOTES: None. PiperOrigin-RevId: 279292100
The + operator on dictionaries will be removed See bazelbuild/bazel#6461 PiperOrigin-RevId: 236656561
See bazelbuild/bazel#6461 PiperOrigin-RevId: 238458969
bazelbuild/bazel#6461 RELNOTES: None. PiperOrigin-RevId: 279292100
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
The text was updated successfully, but these errors were encountered: