google / starlark-go

Starlark in Go: the Starlark configuration language, implemented in Go
BSD 3-Clause "New" or "Revised" License
2.3k stars 209 forks source link

Invalid literal with base 16: int('0b0', 16) #337

Closed IleanaAldama closed 3 years ago

IleanaAldama commented 3 years ago
$ starlark -c "int('0b0', 16)"
Traceback (most recent call last):
  cmdline:1:4: in <toplevel>
Error in int: int: invalid literal with base 16: 0b0
$ python2 -c "print int('0b0', 16)"
176
alandonovan commented 3 years ago

Thanks for the report. This is a tricky case. I think the spec (https://github.com/bazelbuild/starlark/blob/master/spec.md#int) is wrong because it includes this sentence:

These markers [0b etc] may also be used if base is the corresponding base.

This sentence does not appear in the version at https://github.com/google/starlark-go/blob/master/doc/spec.md#int, which I think is correct, and consistent with Python[23]. However, both the Go and Java implementations currently honor the base prefix, even with an explicit base, and if we change that behavior, we may break code that calls int("0x" + s, 16). On the other hand, code that calls int(hexdigits, 16) may today have a latent bug when hexdigits happens to start with 0b.

laurentlb commented 3 years ago

we may break code that calls int("0x" + s, 16)

Python 3 allows both int("0x12", 16) and int("12", 16). It seems to ignore the skip if it matches the base, so I don't see a breaking change here.

I agree we should change the behavior. Is there any visible change here, outside the failure from the original message? If so, I'd like to see code samples.

alandonovan commented 3 years ago

Python 3 allows both int("0x12", 16) and int("12", 16). It seems to ignore the skip if it matches the base, so I don't see a breaking change here.

Oh, that's a relief. Fixed in https://github.com/google/starlark-go/pull/344.