Skip to content
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

Update dict hashability check, once the language spec is updated #8730

Closed
Quarz0 opened this issue Jun 26, 2019 · 2 comments
Closed

Update dict hashability check, once the language spec is updated #8730

Quarz0 opened this issue Jun 26, 2019 · 2 comments
Labels
P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) stale Issues or PRs that are stale (no activity for 30 days) team-Starlark-Interpreter Issues involving the Starlark interpreter used by Bazel type: bug

Comments

@Quarz0
Copy link
Contributor

Quarz0 commented Jun 26, 2019

According to the spec, The meaning of membership varies by the type of the second operand: the members of a list or tuple are its elements; the members of a dict are its keys; the members of a string are all its substrings..

This implies that to use the in operator with dict as a right operand, the left operand has to be a hashable type (like what python does, giving an error if it's not hashable). Also, it'd make no meaning to search for a key that cannot be hashed. Thus the following should give an error rather than False:

[] in {}                 #  False
range(10) in dict()      #  False
{} in {}                 #  False
@laurentlb laurentlb added team-Starlark type: bug P2 We'll consider working on this in future. (Assignee optional) labels Jun 27, 2019
bazel-io pushed a commit that referenced this issue Jul 8, 2019
#8730

Closes #8762.

PiperOrigin-RevId: 256987262
Quarz0 added a commit to Quarz0/bazel that referenced this issue Jul 9, 2019
When the flag is enabled, dict lookup of unhashable keys will fail.
For example using the `in` operator or `dict.get` with unhashabled
types.

RELNOTES: Flag `--incompatible_disallow_dict_lookup_unhashable_keys` is
added. See bazelbuild#8730
bazel-io pushed a commit that referenced this issue Jul 11, 2019
Related: bazelbuild/starlark#65, #8730

Closes #8837.

PiperOrigin-RevId: 257578468
irengrig pushed a commit to irengrig/bazel that referenced this issue Jul 15, 2019
irengrig pushed a commit to irengrig/bazel that referenced this issue Jul 15, 2019
@brandjon
Copy link
Member

The spec wording actually seems to indicate that [] in {} is allowed.

The in operator reports whether its first operand is a member of its second operand
[...]
the members of a dict are its keys

Since [] is not a member of {}, the answer is False.

That said, it's clearly undecided as per bazelbuild/starlark#65. Leaving this issue open to track it from Bazel's side.

@brandjon brandjon added P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) team-Starlark-Interpreter Issues involving the Starlark interpreter used by Bazel and removed P2 We'll consider working on this in future. (Assignee optional) team-Starlark labels Feb 19, 2021
@brandjon brandjon changed the title starlark in operator accepts unhashable types with dict Update dict hashability check, once the language spec is updated Feb 19, 2021
@sgowroji sgowroji added the stale Issues or PRs that are stale (no activity for 30 days) label Feb 15, 2023
@sgowroji
Copy link
Member

Hi there! We're doing a clean up of old issues and will be closing this one. Please reopen if you’d like to discuss anything further. We’ll respond as soon as we have the bandwidth/resources to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P4 This is either out of scope or we don't have bandwidth to review a PR. (No assignee) stale Issues or PRs that are stale (no activity for 30 days) team-Starlark-Interpreter Issues involving the Starlark interpreter used by Bazel type: bug
Projects
None yet
Development

No branches or pull requests

4 participants