feat(string): add parse_bigint (Fixes #3566)#3679
Open
rainhuang0220 wants to merge 4 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR adds parse_bigint as an error-raising string-to-BigInt parser, re-exports it through the string strconv surface, and updates BigInt::from_string to be a panic-on-error wrapper.
Changes:
- Introduce
@bigint.parse_bigint(StringView, base?: Int) -> BigInt raisewith consistent error strings ("invalid syntax","invalid base"). - Re-export
parse_bigintfrom@stringand wire package imports / generated API stubs. - Add unit tests for
parse_bigintin bothbigintandstring.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| string/strconv.mbt | Re-exports parse_bigint via the string strconv API surface. |
| string/pkg.generated.mbti | Adds generated signature for @string.parse_bigint and imports bigint. |
| string/parse_bigint_test.mbt | Adds tests covering @string.parse_bigint basic parsing and error behavior. |
| string/moon.pkg | Adds bigint dependency to support the new re-export. |
| bigint/strconv_errors.mbt | Centralizes parse error messages and helper raisers. |
| bigint/pkg.generated.mbti | Exposes generated signature for @bigint.parse_bigint. |
| bigint/parse_bigint_test.mbt | Adds test coverage for valid/invalid inputs and base validation. |
| bigint/moon.pkg | Adds imports needed for new error raising and tests. |
| bigint/bigint_nonjs.mbt | Implements parse_bigint and refactors existing parsing to raise errors instead of aborting. |
| bigint/bigint_js.mbt | Implements parse_bigint for JS backend and refactors radix parsing to raise errors. |
| bigint/bigint.mbt | Updates JSON decoding to map parse failures into JsonDecodeError. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+25
to
+40
| pub fn parse_bigint(str : StringView, base? : Int = 10) -> BigInt raise { | ||
| if base < 2 || base > 36 { | ||
| base_err() | ||
| } | ||
| if radix < 2 || radix > 36 { | ||
| abort("radix must be between 2 and 36") | ||
| let input = str.to_owned() | ||
| if input.length() == 0 { | ||
| syntax_err() | ||
| } | ||
| if base != 10 { | ||
| return BigInt::from_string_radix(input, base) | ||
| } | ||
| if radix != 10 { | ||
| return BigInt::from_string_radix(str, radix) | ||
| if input == "-" { | ||
| syntax_err() | ||
| } | ||
| if str == "-" { | ||
| abort("invalid character") | ||
| BigInt::js_from_string(input) | ||
| } |
Comment on lines
+1231
to
+1235
| pub fn BigInt::from_string(input : String, radix? : Int = 10) -> BigInt { | ||
| parse_bigint(input.view(), base=radix) catch { | ||
| Failure(msg) => abort(msg) | ||
| _ => abort("invalid syntax") | ||
| } |
Comment on lines
+46
to
50
| pub fn BigInt::from_string(str : String, radix? : Int = 10) -> BigInt { | ||
| parse_bigint(str.view(), base=radix) catch { | ||
| Failure(msg) => abort(msg) | ||
| _ => abort("invalid syntax") | ||
| } |
Comment on lines
+16
to
+21
| test "parse_bigint valid" { | ||
| inspect(@bigint.parse_bigint("12345"), content="12345") | ||
| inspect(@bigint.parse_bigint("-12345"), content="-12345") | ||
| inspect(@bigint.parse_bigint("ff", base=16), content="255") | ||
| inspect(@bigint.parse_bigint("-0"), content="0") | ||
| } |
Comment on lines
+14
to
20
| import { | ||
| "moonbitlang/core/test", | ||
| } for "test" | ||
|
|
||
| import { | ||
| "moonbitlang/core/test", | ||
| } for "wbtest" |
Contributor
Author
|
All checks passed, please review and merge this PR when you have time, thanks. @bobzhang |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
@string.parse_bigintwith raising error handlingBigInt::from_stringpanic behavior for compatibilityBigInt::from_jsonto useparse_bigintFixes [Consistency Review] BigInt::from_string should raise or add a @string.parse_bigint #3566
Test plan
moon test -p bigint -p stringmoon check bigint string --deny-warn