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

rename Char::from_int to Char::from_int_unsafe #338

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions bigint/bigint.mbt
Original file line number Diff line number Diff line change
Expand Up @@ -659,9 +659,9 @@ pub fn to_hex(self : BigInt) -> String {
let y = x % 16
x /= 16
tmp = if y < 10 {
Char::from_int(y + 48).to_string()
Char::from_int(y + 48).unwrap().to_string()
} else {
Char::from_int(y + 55).to_string()
Char::from_int(y + 55).unwrap().to_string()
} + tmp
}
if i != self.len - 1 && tmp.length() < 4 {
Expand Down
1 change: 1 addition & 0 deletions bigint/moon.pkg.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
"import": [
"moonbitlang/core/builtin",
"moonbitlang/core/char",
"moonbitlang/core/option",
"moonbitlang/core/coverage"
],
"test_import": ["moonbitlang/core/assertion"]
Expand Down
2 changes: 1 addition & 1 deletion builtin/builtin.mbti
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ impl Byte {
impl Char {
compare(Char, Char) -> Int
default() -> Char
from_int(Int) -> Char
from_int_unsafe(Int) -> Char
op_equal(Char, Char) -> Bool
to_int(Char) -> Int
}
Expand Down
4 changes: 2 additions & 2 deletions builtin/console.mbt
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ pub fn to_string(self : Int64) -> String {
if num2 != 0L {
write_digits(num2)
}
buf.write_char(Char::from_int(abs(num % 10L).to_int() + 48))
buf.write_char(Char::from_int_unsafe(abs(num % 10L).to_int() + 48))
}
Copy link
Contributor

Choose a reason for hiding this comment

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

is this in perf critical place? if not, we would prefer safe API

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no, but we cannot use Option::unwrap in builtin


write_digits(self)
Expand Down Expand Up @@ -78,7 +78,7 @@ pub fn to_string(self : Int) -> String {
if num2 != 0 {
write_digits(num2)
}
buf.write_char(Char::from_int(abs(num % 10) + 48))
buf.write_char(Char::from_int_unsafe(abs(num % 10) + 48))
}

write_digits(self)
Expand Down
4 changes: 2 additions & 2 deletions builtin/debug.mbt
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,9 @@ pub fn debug_write(self : String, buf : Buffer) -> Unit {

fn to_hex_digit(i : Int) -> Char {
if i < 10 {
Char::from_int('0'.to_int() + i)
Char::from_int_unsafe('0'.to_int() + i)
} else {
Char::from_int('a'.to_int() + (i - 10))
Char::from_int_unsafe('a'.to_int() + (i - 10))
}
}

Expand Down
3 changes: 2 additions & 1 deletion builtin/intrinsics.mbt
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,8 @@ pub fn Double::convert_i64_u(val : Int64) -> Double = "%i64_to_f64_u"

pub fn Char::to_int(self : Char) -> Int = "%char_to_int"

pub fn Char::from_int(val : Int) -> Char = "%char_from_int"
// Converts a Int to a Char, ignoring validity.
pub fn Char::from_int_unsafe(val : Int) -> Char = "%char_from_int"

Copy link
Contributor

Choose a reason for hiding this comment

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

this unsafe API does not be exposed, it can be pulled in on demand, so that it is only local to its used package

Copy link
Contributor Author

@lijunchen lijunchen May 14, 2024

Choose a reason for hiding this comment

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

In my current implementation, it is used by coverage and json5. I think exposing Char::from_int_unsafe is reasonable.

pub fn Char::op_equal(self : Char, other : Char) -> Bool = "%char_eq"

Expand Down
48 changes: 44 additions & 4 deletions char/char.mbt
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,9 @@ test "to_string" {
pub fn debug_write(self : Char, buf : Buffer) -> Unit {
fn to_hex_digit(i : Int) -> Char {
if i < 10 {
Char::from_int('0'.to_int() + i)
Char::from_int('0'.to_int() + i).unwrap()
} else {
Char::from_int('a'.to_int() + (i - 10))
Char::from_int('a'.to_int() + (i - 10)).unwrap()
}
}

Expand Down Expand Up @@ -61,7 +61,7 @@ pub fn debug_write(self : Char, buf : Buffer) -> Unit {

test "debug_write" {
fn repr(chr) {
let buf = Buffer::make(0)
let buf = Buffer::make(4)
debug_write(chr, buf)
buf.to_string()
}
Expand All @@ -74,9 +74,49 @@ test "debug_write" {
@assertion.assert_eq(repr('\r'), "'\\r'")?
@assertion.assert_eq(repr('\b'), "'\\b'")?
@assertion.assert_eq(repr('\t'), "'\\t'")?
@assertion.assert_eq(repr(Char::from_int(0)), "'\\x00'")?
@assertion.assert_eq(repr('\x0f'), "'\\x0f'")?
@assertion.assert_eq(repr(Char::from_int_unsafe(0)), "'\\x00'")?
}

pub fn hash(self : Char) -> Int {
self.to_int()
}

// The lowest valid code point a char can have, '\u{0}'.
pub let min : Char = '\u{0}'

lijunchen marked this conversation as resolved.
Show resolved Hide resolved
// The highest valid code point a char can have, '\u{10FFFF}'.
pub let max : Char = '\u{10FFFF}'

let lo_bound : Char = '\u{D7FF}'

let hi_bound : Char = '\u{E000}'

pub fn is_valid(self : Char) -> Bool {
min <= self && self <= lo_bound || hi_bound <= self && self <= max
}

// Converts a Int to a Char.
pub fn Char::from_int(i : Int) -> Option[Char] {
let c = Char::from_int_unsafe(i)
if c.is_valid() {
Some(c)
} else {
None
}
}

test "invalid" {
inspect(Char::from_int_unsafe(0xD800).is_valid(), content="false")?
inspect(Char::from_int(0xD800), content="None")?
inspect(Char::from_int(-1), content="None")?
}

test "Unicode" {
inspect(Char::from_int_unsafe(97), content="a")?
inspect('中'.to_int(), content="20013")?
inspect(Char::from_int(20013), content="Some(中)")?
inspect(Char::from_int_unsafe(20013), content="中")?
inspect('🤣'.to_int(), content="129315")?
inspect(Char::from_int(129315), content="Some(🤣)")?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Guest0x0 Some(🤣) should be Some('🤣')

}
5 changes: 5 additions & 0 deletions char/char.mbti
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
package moonbitlang/core/char

// Values
let max : Char

let min : Char

// Types and methods
impl Char {
debug_write(Char, Buffer) -> Unit
from_int(Int) -> Option[Char]
hash(Char) -> Int
is_valid(Char) -> Bool
to_string(Char) -> String
}

Expand Down
1 change: 1 addition & 0 deletions char/moon.pkg.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
"import": [
"moonbitlang/core/builtin",
"moonbitlang/core/assertion",
"moonbitlang/core/option",
"moonbitlang/core/coverage"
]
}
4 changes: 2 additions & 2 deletions coverage/coverage.mbt
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,9 @@ fn escape_to(s : String, buf : Buffer) -> Unit {

fn to_hex_digit(i : Int) -> Char {
if i < 10 {
Char::from_int('0'.to_int() + i)
Char::from_int_unsafe('0'.to_int() + i)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

cannot use Option::unwrap here, because of cyclic dependencies

} else {
Char::from_int('a'.to_int() + (i - 10))
Char::from_int_unsafe('a'.to_int() + (i - 10))
}
}

Expand Down
3 changes: 2 additions & 1 deletion double/moon.pkg.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
{
"import": [
"moonbitlang/core/builtin",

"moonbitlang/core/char",
"moonbitlang/core/option",
"moonbitlang/core/assertion",
"moonbitlang/core/bool",
"moonbitlang/core/int64",
Expand Down
2 changes: 1 addition & 1 deletion double/ryu.mbt
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ fn log10Pow2(e : Int) -> Int {
fn string_from_bytes(bytes : Bytes, from : Int, to : Int) -> String {
let buf = Buffer::make(bytes.length())
for i = from; i < to; i = i + 1 {
buf.write_char(Char::from_int(bytes[i]))
buf.write_char(Char::from_int(bytes[i]).unwrap())
}
buf.to_string()
}
Expand Down
4 changes: 2 additions & 2 deletions iter/iter_test.mbt
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ test "filter" {
test "map" {
let iter = from_array(['1', '2', '3', '4', '5'])
let exb = Buffer::make(0)
iter.map(fn { x => Char::from_int(x.to_int() + 1) }).iter(
iter.map(fn { x => Char::from_int(x.to_int() + 1).unwrap() }).iter(
fn { x => exb.write_char(x) },
)
exb.expect(content="23456")?
Expand All @@ -176,7 +176,7 @@ test "flat_map2" {

test "fold" {
let iter = from_array(['1', '2', '3', '4', '5'])
let result = Char::from_int(
let result = Char::from_int_unsafe(
iter.fold(fn { acc, x => acc + x.to_int() }, 0) / 5,
)
@assertion.assert_eq(result, '3')?
Expand Down
3 changes: 2 additions & 1 deletion iter/moon.pkg.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
"moonbitlang/core/coverage"
],
"test_import": [
"moonbitlang/core/char"
"moonbitlang/core/char",
"moonbitlang/core/option"
]
}
2 changes: 1 addition & 1 deletion json5/lex_misc.mbt
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ fn read_char(ctx : ParseContext) -> Option[Char] {
if c2 >= 0xDC00 && c2 <= 0xDFFF {
ctx.offset += 1
let c3 = c1.lsl(10) + c2 - 0x35fdc00
return Some(Char::from_int(c3))
return Some(Char::from_int_unsafe(c3))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

maybe performance critical in json5 package

}
}
}
Expand Down
4 changes: 2 additions & 2 deletions json5/lex_prop.mbt
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ fn lex_property_name(ctx : ParseContext) -> Result[Token, ParseError] {
Some('\\') => {
lex_assert_char(ctx, 'u')?
let c = lex_hex_digits(ctx, 4)?
let c = Char::from_int(c)
let c = Char::from_int_unsafe(c)
if c == '$' || c == '_' || c >= 'a' && c <= 'z' || c >= 'A' && c <= 'Z' ||
c > '\x7f' && non_ascii_id_start.contains(c) {
let buffer = StringBuilder::make()
Expand Down Expand Up @@ -69,7 +69,7 @@ fn lex_ident(
flush(ctx.offset - 1)
lex_assert_char(ctx, 'u')?
let c = lex_hex_digits(ctx, 4)?
let c = Char::from_int(c)
let c = Char::from_int_unsafe(c)
if c == '$' || c == '_' || c >= 'a' && c <= 'z' || c >= 'A' && c <= 'Z' ||
c >= '0' && c <= '9' || c > '\x7f' && non_ascii_id_continue.contains(c) {
buffer.add_char(c)
Expand Down
4 changes: 2 additions & 2 deletions json5/lex_string.mbt
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,11 @@ fn lex_string(ctx : ParseContext, quote : Char) -> Result[String, ParseError] {
}
Some('x') => {
let c = lex_hex_digits(ctx, 2)?
buf.add_char(Char::from_int(c))
buf.add_char(Char::from_int_unsafe(c))
}
Some('u') => {
let c = lex_hex_digits(ctx, 4)?
buf.add_char(Char::from_int(c))
buf.add_char(Char::from_int_unsafe(c))
}
Some(c) => {
if c >= '1' && c <= '9' {
Expand Down
3 changes: 2 additions & 1 deletion strconv/moon.pkg.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
"moonbitlang/core/tuple",
"moonbitlang/core/char",
"moonbitlang/core/string",
"moonbitlang/core/int64"
"moonbitlang/core/int64",
"moonbitlang/core/option"
]
}
2 changes: 1 addition & 1 deletion strconv/string_slice.mbt
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ fn prefix_eq_ignore_case(self : StringSlice, s2 : String) -> Bool {

fn lower(c : Char) -> Char {
if 'A' <= c && c <= 'Z' {
Char::from_int(c.to_int() + 'a'.to_int() - 'A'.to_int())
Char::from_int(c.to_int() + 'a'.to_int() - 'A'.to_int()).unwrap()
} else {
c
}
Expand Down
3 changes: 2 additions & 1 deletion string/moon.pkg.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
"moonbitlang/core/coverage",
"moonbitlang/core/bytes",
"moonbitlang/core/char",
"moonbitlang/core/iter"
"moonbitlang/core/iter",
"moonbitlang/core/option"
]
}
4 changes: 2 additions & 2 deletions string/string.mbt
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@
if c >= 0xD800 && c <= 0xDBFF && index + 1 < len {
let c2 = self[index + 1].to_int()
if c2 >= 0xDC00 && c2 <= 0xDFFF {
let c = Char::from_int((c - 0xD800) * 0x400 + c2 - 0xDC00 + 0x10000)
let c = Char::from_int((c - 0xD800) * 0x400 + c2 - 0xDC00 + 0x10000).unwrap()
if yield(c) {
continue index + 2
} else {
Expand All @@ -168,7 +168,7 @@
}
}
//TODO: handle garbage input
if not(yield(Char::from_int(c))) {
if not(yield(Char::from_int(c).unwrap())) {

Check warning on line 171 in string/string.mbt

View check run for this annotation

Codecov / codecov/patch

string/string.mbt#L171

Added line #L171 was not covered by tests
break false
}
} else {
Expand Down