Skip to content

fix with semantics#42

Merged
mshinwell merged 1 commit intomshinwell:5.2.0-parallelfrom
riaqn:liam-with-syntax
Sep 11, 2024
Merged

fix with semantics#42
mshinwell merged 1 commit intomshinwell:5.2.0-parallelfrom
riaqn:liam-with-syntax

Conversation

@riaqn
Copy link
Copy Markdown

@riaqn riaqn commented Sep 10, 2024

No description provided.

@riaqn riaqn changed the title Support with fix with semantics Sep 10, 2024
@mshinwell mshinwell marked this pull request as ready for review September 11, 2024 09:24
@mshinwell mshinwell merged commit 2190b34 into mshinwell:5.2.0-parallel Sep 11, 2024
mshinwell pushed a commit that referenced this pull request Sep 11, 2025
This feature adds literals for the remaining integer types:
* `int8`: `42s`,
* `int16`: `42S`,
* `int8#`: `#42s`,
* `int16#`: `#42S`
* and `int#`: `#42m`.

The literal syntax `#42` is not an option because it causes an ambiguity with line number directives. The literal syntax `42m` for regular tagged integers was also added to mirror the untagged immediate literals.

The changes are mostly straightforward since we don't need to modify the parser. The one rough edge is converting strings to small ints. Since small ints are represented as regular `int`s in the compiler, we could just call `int_of_string` to convert, but this would not properly handle overflow[^1]. Ideally, we would call the C primitive `parse_intnat` to handle most of the overflow logic, but it takes some of its arguments unboxed. A less ideal but feasible option is to put a C stub in `utils/` that calls `parse_intnat`, but this would be awkward since it would be the only C stub used during typing. The option I chose to go with is to reimplement the overflow logic in OCaml (in `utils/misc.ml`). Extensive testing of this overflow logic is found in `testsuite/tests/typing/small-numbers/test_enabled.ml`.

The choice of `m` for the untagged immediate suffix is a bit arbitrary. The main consideration is that the suffix isn't overloaded (e.g. `#42u` would be a bad choice since it could be confused for an unsigned literal).

## Reviewing

Start by looking at the tests. In particular, make sure every case of overflowing literals is tested and has behavior similar to regular ints.

Most of the changes in the source are copy-pastes of code that handles the other integer types, so reviewing should be easy. Two places require extra care:
* In `utils/misc.ml`, make sure `cvt_small_int` handles overflow correctly (see long paragraph above)
* In the printers, make sure that every time that some literals are printed with a suffix, untagged immediates are also printed with a suffix (in an earlier version of this PR, untagged immediates were not given a suffix).

[^1]: Handling overflow is more complex than you'd expect: see <ocaml/ocaml#4210>.

---------

Co-authored-by: James Rayman <james@jamesrayman.com>
mshinwell pushed a commit that referenced this pull request Sep 12, 2025
This feature adds literals for the remaining integer types:
* `int8`: `42s`,
* `int16`: `42S`,
* `int8#`: `#42s`,
* `int16#`: `#42S`
* and `int#`: `#42m`.

The literal syntax `#42` is not an option because it causes an ambiguity with line number directives. The literal syntax `42m` for regular tagged integers was also added to mirror the untagged immediate literals.

The changes are mostly straightforward since we don't need to modify the parser. The one rough edge is converting strings to small ints. Since small ints are represented as regular `int`s in the compiler, we could just call `int_of_string` to convert, but this would not properly handle overflow[^1]. Ideally, we would call the C primitive `parse_intnat` to handle most of the overflow logic, but it takes some of its arguments unboxed. A less ideal but feasible option is to put a C stub in `utils/` that calls `parse_intnat`, but this would be awkward since it would be the only C stub used during typing. The option I chose to go with is to reimplement the overflow logic in OCaml (in `utils/misc.ml`). Extensive testing of this overflow logic is found in `testsuite/tests/typing/small-numbers/test_enabled.ml`.

The choice of `m` for the untagged immediate suffix is a bit arbitrary. The main consideration is that the suffix isn't overloaded (e.g. `#42u` would be a bad choice since it could be confused for an unsigned literal).

## Reviewing

Start by looking at the tests. In particular, make sure every case of overflowing literals is tested and has behavior similar to regular ints.

Most of the changes in the source are copy-pastes of code that handles the other integer types, so reviewing should be easy. Two places require extra care:
* In `utils/misc.ml`, make sure `cvt_small_int` handles overflow correctly (see long paragraph above)
* In the printers, make sure that every time that some literals are printed with a suffix, untagged immediates are also printed with a suffix (in an earlier version of this PR, untagged immediates were not given a suffix).

[^1]: Handling overflow is more complex than you'd expect: see <ocaml/ocaml#4210>.

---------

Co-authored-by: James Rayman <james@jamesrayman.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants