Skip to content

uint16 overflow check #1991

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

uint16 overflow check #1991

wants to merge 1 commit into from

Conversation

bobzhang
Copy link
Contributor

@bobzhang bobzhang commented Apr 23, 2025

there are two things wrong:

  • literal overflow check missed
  • overflow behavior inconsistent between different backends, the native backend wrap to 0 seems to be reasonable

@bobzhang bobzhang force-pushed the hongbo/uint16_overflow branch from 1691a8c to 639a3b5 Compare April 23, 2025 06:50
Copy link

Test case attempts to create UInt16 with value that exceeds maximum possible value

Category
Correctness
Code Snippet
inspect!((65536 : UInt16), content="65536")
Recommendation
This test should remain commented out or be modified to test overflow behavior explicitly. If testing overflow handling is intended, it should be in a separate test case clearly marked as testing overflow behavior.
Reasoning
UInt16 can only hold values from 0 to 65535 (2^16 - 1). The value 65536 exceeds this range and would cause an overflow. The original code correctly had this commented out with an explanatory comment.

@coveralls
Copy link
Collaborator

coveralls commented Apr 23, 2025

Pull Request Test Coverage Report for Build 6396

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 93.238%

Totals Coverage Status
Change from base Build 6394: 0.0%
Covered Lines: 5860
Relevant Lines: 6285

💛 - Coveralls

@bobzhang
Copy link
Contributor Author

changed to

  inspect!((65535 : UInt16) + 1, content="65536")

to test overflow behavior
compiler crashed

Error: Moonc.Basic_hash_string.Key_not_found("$@moonbitlang/core/builtin.Add::UInt16::op_add")

cc @Guest0x0

@Yu-zh
Copy link
Collaborator

Yu-zh commented Apr 28, 2025

changed to

  inspect!((65535 : UInt16) + 1, content="65536")

to test overflow behavior compiler crashed

Error: Moonc.Basic_hash_string.Key_not_found("$@moonbitlang/core/builtin.Add::UInt16::op_add")

cc @Guest0x0

Add "moonbitlang/core/uint16" to "test-import" will fix this problem. This problem only happens when you are developing core

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