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

SliceSuffix is inconsistent #291

Open
lgalfaso opened this issue Nov 24, 2024 · 2 comments
Open

SliceSuffix is inconsistent #291

lgalfaso opened this issue Nov 24, 2024 · 2 comments

Comments

@lgalfaso
Copy link
Contributor

The spec defines

SliceSuffix = '[' [Expression] ':' [Test] [':' [Test]] ']'
            | '[' Expression ']'
            .

Expression = Test {',' Test} .

This would imply that the following is a valid Starlark program

a = {}
a[1,2:3] = "foo"
print(a)

When running this program in Python, the output is the following

{(1, slice(2, 3, None)): 'foo'}

When running the same program in Bazel, the output is the following error

cannot assign to 'a[(1, 2):3]'

There are the following aspects around the current syntax:

  1. In Python parses this as a tuple with the first element an int and the second a slice. Starlark is a slice with the first element a tuple and the second an int.
  2. If Bazel does not allow setting a value to a dict with the key being a slice (making the assumption that the previous point was intentional), why is the syntax not '[' [Test] ':' [Test] [':' [Test]] ']' ? Is there some other case that the generality of the syntax is needed?
@brandjon
Copy link
Member

Thanks for spotting this ambiguity. I can't repro in Python 3.11.9 because it complains that slices aren't hashable, but inspecting the tree obtained from ast.parse('a[1,2:3]') confirms it's a tuple who second item is a slice.

Personally I'd be in favor of banning this example as ambiguous / likely to cause confusion. Unlike Python, the Starlark concept of a slice is just a syntactic construct, not a reified runtime object, and I don't think the Python behavior can even be expressed without changing that.

It sounds like this is resolved by changing Expression to Test as you suggest. @adonovan, any concerns?

@lgalfaso
Copy link
Contributor Author

lgalfaso commented Jan 6, 2025

The code in the original comment was tested with Python 3.13.1 on MacOS, and works as stated there. In this version of Python, it does not run into the issue present at #291 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants