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

Inconsistent parsing and/or evaluation for negative u64. #1161

Closed
porcuquine opened this issue Feb 21, 2024 · 1 comment
Closed

Inconsistent parsing and/or evaluation for negative u64. #1161

porcuquine opened this issue Feb 21, 2024 · 1 comment
Assignees
Labels

Comments

@porcuquine
Copy link
Collaborator

porcuquine commented Feb 21, 2024

Something is wrong with the reading and subsequent handling of negative (Lurk) u64s.

A naked read yields a result obviously larger than u64. This result appears to just be the corresponding Num (see example below).

However, attempting to perform a computation on the expression yields an error — whereas using the corresponding Num does not.

I suspect we are creating an illegal value — perhaps tagged as u64 but with a value out of range for that type.

Most importantly, this behavior should be consistent. Intuitively, although a bit weird, -1u64 should probably just be syntax for 0xFFFFFFFFFFFFFFFF. That would be the most useful result. Alternately, we might decide this should be a parsing error.

Whatever we do, the current state is a bug.

commit: 2024-02-13 1d4d308e2bc12f5ab431ea210c0b722f9eb31825
Lurk REPL welcomes you.
user> -1u64
[1 iteration] => 0x30644e72e131a029b85045b68181585d2833e84879b9709143e1f593f0000000
user> -1
[1 iteration] => 0x30644e72e131a029b85045b68181585d2833e84879b9709143e1f593f0000000
user> (- 0u64 1u64)
[3 iterations] => 18446744073709551615u64
user> (+ (- 0u64 1u64) 10u64)
[6 iterations] => 9u64
user> (+ -1u64 10u64)
Error: Evaluation encountered an error after 2 iterations
user> (+ -1 10u64)
[3 iterations] => 9
@porcuquine porcuquine added bug Something isn't working B-3 Lurk usability & testing labels Feb 21, 2024
johnchandlerburnham added a commit that referenced this issue Feb 24, 2024
- merges previous parse_num and parse_uint into a single parse_numeric
- removes bounds check on Num, so that parsing numbers larger than the
  field will modularly wrap
- corrects uint parsing so that we never accidentally parse uints as
  nums
- implements a placeholder syntax for i64 for parsing twos-complement
  numbers (i.e. -1i64 parses to u64::MAX). In future this syntax should
  parse to a new literal type
- add placeholder parsing for u8, u16, u32, u128, i8, i16, i32, i128
  literals so that we correctly error that these have yet to be
  implemented
- adds unit tests to ensure correctness of the above
github-merge-queue bot pushed a commit that referenced this issue Feb 25, 2024
* new numeric parser, closes #609, #1161, #1169

- merges previous parse_num and parse_uint into a single parse_numeric
- removes bounds check on Num, so that parsing numbers larger than the
  field will modularly wrap
- corrects uint parsing so that we never accidentally parse uints as
  nums
- implements a placeholder syntax for i64 for parsing twos-complement
  numbers (i.e. -1i64 parses to u64::MAX). In future this syntax should
  parse to a new literal type
- add placeholder parsing for u8, u16, u32, u128, i8, i16, i32, i128
  literals so that we correctly error that these have yet to be
  implemented
- adds unit tests to ensure correctness of the above

* clippy

* clippy part II: revenge of the linter

* remove unused byte parsers and associated tests

* comment out printlns
@arthurpaulino
Copy link
Member

Closed by #1171

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

No branches or pull requests

2 participants