Skip to content

Conversation

paurkedal
Copy link
Collaborator

This includes #43, my extra integer conversions, and rebases it on the current master branch.

mfp and others added 4 commits April 10, 2025 08:45
  - Field.int: Accept conversions from 64 bit integers. This can
    overflow, but on 64 bit platforms there is a large usable range
    which has been accepted in the past. We may consider adding overflow
    detection, though.
  - Field.int64: Accept conversions from smaller integers.
@jongleb
Copy link
Contributor

jongleb commented Sep 8, 2025

Hello, Could you please suggest if you are going to merge this?

Comment on lines 111 to 118
| `Int64 i, `Int x | `Int x, `Int64 i -> Int64.(equal i (of_int x))
| `Int64 i, `Float x | `Float x, `Int64 i -> Int64.to_float i = x
| `UInt64 i, `Int x | `Int x, `UInt64 i -> Unsigned.UInt64.(equal i (of_int x))
| `UInt64 i, `Float x | `Float x, `UInt64 i -> Int64.to_float (Unsigned.UInt64.to_int64 i) = x
| `UInt64 i, `Int64 x | `Int64 x, `UInt64 i -> Int64.equal (Unsigned.UInt64.to_int64 i) x
| `Int _, _ | _, `Int _ -> false
| `Int64 _, _ | _, `Int64 _ -> false
| `UInt64 _, _ | _, `UInt64 _ -> false
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks you like are missing `Int64 _, `Int64 _ and `UInt64 _, `UIint64 _ cases here.

@paurkedal
Copy link
Collaborator Author

Looks good, and I can see I already tentatively approved it in my comment to #43 Apr 10, so let's merge it, but please have a look at my comment on equal_field first.

Gleb Patsiia and others added 2 commits September 24, 2025 08:41
`Int64 _, `Int64 _ and  `UInt64 _, `UIint64 _
@paurkedal paurkedal merged commit e9f9ac7 into ocaml-community:master Sep 25, 2025
5 of 6 checks passed
@paurkedal
Copy link
Collaborator Author

Thanks for the contribution!

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.

3 participants