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

value_to_usize loses precision #76

Open
bnjbvr opened this issue Jan 10, 2023 · 2 comments
Open

value_to_usize loses precision #76

bnjbvr opened this issue Jan 10, 2023 · 2 comments
Labels
bug Something isn't working

Comments

@bnjbvr
Copy link
Contributor

bnjbvr commented Jan 10, 2023

On 32-bits architectures, usize is 32-bits, by definition, so any conversion from a larger type into this will drop some bits. On 64-bits architecture, converting from 128-bits to 64-bits will also lose bits. It seems that it's only used for key paths computations, so for instance on 64-bits, that would require a static access in a list at an index >= 2**64 to cause an issue, so it's rather unlikely to happen, but it's still a real bug.

@bnjbvr bnjbvr added the bug Something isn't working label Jan 10, 2023
@davidpdrsn
Copy link
Member

Probably worth testing if we can change that function to only handle usize and not cast the type integer types, like it does now. So only return Some for Value::usize.

I wrote it like this originally because I was worried about integer literals defaulting to i32 would be an issue but I haven't actually tested that.

@davidpdrsn
Copy link
Member

We could also just panic if a lossless conversion fails. Like you say hitting that requires accessing very large lists.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants