From b7420523ea79be9b1e83ca2fd0815f981a4a40f0 Mon Sep 17 00:00:00 2001 From: Spotandjake Date: Tue, 13 Feb 2024 22:46:38 -0500 Subject: [PATCH 1/4] chore(compiler): Add `fromNumber()` warning for `short` types --- compiler/src/typed/typed_well_formedness.re | 29 ++++--- compiler/src/utils/warnings.re | 91 ++++++++++----------- compiler/src/utils/warnings.rei | 19 +++-- compiler/test/suites/numbers.re | 16 ++-- 4 files changed, 84 insertions(+), 71 deletions(-) diff --git a/compiler/src/typed/typed_well_formedness.re b/compiler/src/typed/typed_well_formedness.re index ac6956737e..128c6fef6f 100644 --- a/compiler/src/typed/typed_well_formedness.re +++ b/compiler/src/typed/typed_well_formedness.re @@ -276,7 +276,7 @@ module WellFormednessArg: TypedtreeIter.IteratorArgument = { Grain_parsing.Location.prerr_warning(exp_loc, warning); }; } - // Check: Warn if using Int32.fromNumber() + // Check: Warn if using XXXX.fromNumber() | TExpApp( { exp_desc: @@ -289,7 +289,7 @@ module WellFormednessArg: TypedtreeIter.IteratorArgument = { _, [ ( - Unlabeled, + _, { exp_desc: TExpConstant( @@ -302,8 +302,12 @@ module WellFormednessArg: TypedtreeIter.IteratorArgument = { ], ) when - modname == "Int32" + modname == "Int8" + || modname == "Int16" + || modname == "Int32" || modname == "Int64" + || modname == "Uint8" + || modname == "Uint16" || modname == "Uint32" || modname == "Uint64" || modname == "Float32" @@ -315,16 +319,21 @@ module WellFormednessArg: TypedtreeIter.IteratorArgument = { | Const_number_float(nfloat) => Float.to_string(nfloat) | _ => failwith("Impossible") }; - let warning = + let mod_type = switch (modname) { - | "Int32" => Grain_utils.Warnings.FromNumberLiteralI32(n_str) - | "Int64" => Grain_utils.Warnings.FromNumberLiteralI64(n_str) - | "Uint32" => Grain_utils.Warnings.FromNumberLiteralU32(n_str) - | "Uint64" => Grain_utils.Warnings.FromNumberLiteralU64(n_str) - | "Float32" => Grain_utils.Warnings.FromNumberLiteralF32(n_str) - | "Float64" => Grain_utils.Warnings.FromNumberLiteralF64(n_str) + | "Int8" => Grain_utils.Warnings.Int8 + | "Int16" => Grain_utils.Warnings.Int16 + | "Int32" => Grain_utils.Warnings.Int32 + | "Int64" => Grain_utils.Warnings.Int64 + | "Uint8" => Grain_utils.Warnings.Uint8 + | "Uint16" => Grain_utils.Warnings.Uint16 + | "Uint32" => Grain_utils.Warnings.Uint32 + | "Uint64" => Grain_utils.Warnings.Uint64 + | "Float32" => Grain_utils.Warnings.Float32 + | "Float64" => Grain_utils.Warnings.Float64 | _ => failwith("Impossible") }; + let warning = Grain_utils.Warnings.FromNumberLiteral(mod_type, n_str); if (Grain_utils.Warnings.is_active(warning)) { Grain_parsing.Location.prerr_warning(exp_loc, warning); }; diff --git a/compiler/src/utils/warnings.re b/compiler/src/utils/warnings.re index 8a00138cd6..a641b9741f 100644 --- a/compiler/src/utils/warnings.re +++ b/compiler/src/utils/warnings.re @@ -6,6 +6,18 @@ type loc = { loc_ghost: bool, }; +type number_type = + | Int8 + | Int16 + | Int32 + | Int64 + | Uint8 + | Uint16 + | Uint32 + | Uint64 + | Float32 + | Float64; + type t = | LetRecNonFunction(string) | AmbiguousName(list(string), list(string), bool) @@ -24,15 +36,10 @@ type t = | ShadowConstructor(string) | NoCmiFile(string, option(string)) | FuncWasmUnsafe(string, string, string) - | FromNumberLiteralI32(string) - | FromNumberLiteralI64(string) - | FromNumberLiteralU32(string) - | FromNumberLiteralU64(string) - | FromNumberLiteralF32(string) - | FromNumberLiteralF64(string) + | FromNumberLiteral(number_type, string) | UselessRecordSpread; -let last_warning_number = 24; +let last_warning_number = 19; let number = fun @@ -53,12 +60,7 @@ let number = | NonClosedRecordPattern(_) => 15 | UnusedExtension => 16 | FuncWasmUnsafe(_, _, _) => 17 - | FromNumberLiteralI32(_) => 18 - | FromNumberLiteralI64(_) => 19 - | FromNumberLiteralU32(_) => 20 - | FromNumberLiteralU64(_) => 21 - | FromNumberLiteralF32(_) => 22 - | FromNumberLiteralF64(_) => 23 + | FromNumberLiteral(_, _) => 18 | UselessRecordSpread => last_warning_number; let message = @@ -127,36 +129,32 @@ let message = ++ " from the `" ++ m ++ "` module the instead." - | FromNumberLiteralI32(n) => - Printf.sprintf( - "it looks like you are calling Int32.fromNumber() with a constant number. Try using the literal syntax (e.g. `%sl`) instead.", - n, - ) - | FromNumberLiteralI64(n) => - Printf.sprintf( - "it looks like you are calling Int64.fromNumber() with a constant number. Try using the literal syntax (e.g. `%sL`) instead.", - n, - ) - | FromNumberLiteralU32(n) => - Printf.sprintf( - "it looks like you are calling Uint32.fromNumber() with a constant number. Try using the literal syntax (e.g. `%sul`) instead.", - n, - ) - | FromNumberLiteralU64(n) => - Printf.sprintf( - "it looks like you are calling Uint64.fromNumber() with a constant number. Try using the literal syntax (e.g. `%suL`) instead.", - n, - ) - | FromNumberLiteralF32(n) => - Printf.sprintf( - "it looks like you are calling Float32.fromNumber() with a constant number. Try using the literal syntax (e.g. `%sf`) instead.", - String.contains(n, '.') ? n : n ++ ".", - ) - | FromNumberLiteralF64(n) => - Printf.sprintf( - "it looks like you are calling Float64.fromNumber() with a constant number. Try using the literal syntax (e.g. `%sd`) instead.", - String.contains(n, '.') ? n : n ++ ".", - ) + | FromNumberLiteral(mod_type, n) => { + let (mod_name, literal) = + switch (mod_type) { + | Int8 => ("Int8", Printf.sprintf("%ss", n)) + | Int16 => ("Int16", Printf.sprintf("%sS", n)) + | Int32 => ("Int32", Printf.sprintf("%sl", n)) + | Int64 => ("Int64", Printf.sprintf("%sL", n)) + | Uint8 => ("Uint8", Printf.sprintf("%sus", n)) + | Uint16 => ("Uint16", Printf.sprintf("%suS", n)) + | Uint32 => ("Uint32", Printf.sprintf("%sul", n)) + | Uint64 => ("Uint64", Printf.sprintf("%suL", n)) + | Float32 => ( + "Float32", + Printf.sprintf("%sf", String.contains(n, '.') ? n : n ++ "."), + ) + | Float64 => ( + "Float64", + Printf.sprintf("%sd", String.contains(n, '.') ? n : n ++ "."), + ) + }; + Printf.sprintf( + "it looks like you are calling %s.fromNumber() with a constant number. Try using the literal syntax (e.g. `%s`) instead.", + mod_name, + literal, + ); + } | UselessRecordSpread => "this record spread is useless as all of the record's fields are overridden."; let sub_locs = @@ -200,12 +198,7 @@ let defaults = [ ShadowConstructor(""), NoCmiFile("", None), FuncWasmUnsafe("", "", ""), - FromNumberLiteralI32(""), - FromNumberLiteralI64(""), - FromNumberLiteralU32(""), - FromNumberLiteralU64(""), - FromNumberLiteralF32(""), - FromNumberLiteralF64(""), + FromNumberLiteral(Int8, ""), UselessRecordSpread, ]; diff --git a/compiler/src/utils/warnings.rei b/compiler/src/utils/warnings.rei index 0cc64e6503..667d962085 100644 --- a/compiler/src/utils/warnings.rei +++ b/compiler/src/utils/warnings.rei @@ -20,6 +20,18 @@ type loc = { loc_ghost: bool, }; +type number_type = + | Int8 + | Int16 + | Int32 + | Int64 + | Uint8 + | Uint16 + | Uint32 + | Uint64 + | Float32 + | Float64; + type t = | LetRecNonFunction(string) | AmbiguousName(list(string), list(string), bool) @@ -38,12 +50,7 @@ type t = | ShadowConstructor(string) | NoCmiFile(string, option(string)) | FuncWasmUnsafe(string, string, string) - | FromNumberLiteralI32(string) - | FromNumberLiteralI64(string) - | FromNumberLiteralU32(string) - | FromNumberLiteralU64(string) - | FromNumberLiteralF32(string) - | FromNumberLiteralF64(string) + | FromNumberLiteral(number_type, string) | UselessRecordSpread; let is_active: t => bool; diff --git a/compiler/test/suites/numbers.re b/compiler/test/suites/numbers.re index deea93f564..e678e99df2 100644 --- a/compiler/test/suites/numbers.re +++ b/compiler/test/suites/numbers.re @@ -200,32 +200,36 @@ describe("numbers", ({test, testSkip}) => { // well-formedness warnings test("float32_fromNumber_warn1", ({expect}) => { - expect.string(Warnings.message(FromNumberLiteralF32("5"))).toMatch( + expect.string(Warnings.message(FromNumberLiteral(Float32, "5"))).toMatch( "5.f", ) }); test("float32_fromNumber_warn2", ({expect}) => { - expect.string(Warnings.message(FromNumberLiteralF32("5."))).toMatch( + expect.string(Warnings.message(FromNumberLiteral(Float32, "5."))). + toMatch( "5.f", ) }); test("float32_fromNumber_warn3", ({expect}) => { - expect.string(Warnings.message(FromNumberLiteralF32("5.5"))).toMatch( + expect.string(Warnings.message(FromNumberLiteral(Float32, "5.5"))). + toMatch( "5.5f", ) }); test("float64_fromNumber_warn1", ({expect}) => { - expect.string(Warnings.message(FromNumberLiteralF64("5"))).toMatch( + expect.string(Warnings.message(FromNumberLiteral(Float64, "5"))).toMatch( "5.d", ) }); test("float64_fromNumber_warn2", ({expect}) => { - expect.string(Warnings.message(FromNumberLiteralF64("5."))).toMatch( + expect.string(Warnings.message(FromNumberLiteral(Float64, "5."))). + toMatch( "5.d", ) }); test("float64_fromNumber_warn3", ({expect}) => { - expect.string(Warnings.message(FromNumberLiteralF64("5.5"))).toMatch( + expect.string(Warnings.message(FromNumberLiteral(Float64, "5.5"))). + toMatch( "5.5d", ) }); From 470f823725f408e3fd6a01cb279441f644240571 Mon Sep 17 00:00:00 2001 From: Spotandjake Date: Sun, 18 Feb 2024 18:13:46 -0500 Subject: [PATCH 2/4] chore: Support `Rational` and `Bigint` --- compiler/src/typed/typed_well_formedness.re | 19 +++++++-- compiler/src/utils/warnings.re | 46 +++++++++++---------- compiler/src/utils/warnings.rei | 6 ++- compiler/test/suites/numbers.re | 26 +++++++++--- 4 files changed, 63 insertions(+), 34 deletions(-) diff --git a/compiler/src/typed/typed_well_formedness.re b/compiler/src/typed/typed_well_formedness.re index 128c6fef6f..0e0b4cf839 100644 --- a/compiler/src/typed/typed_well_formedness.re +++ b/compiler/src/typed/typed_well_formedness.re @@ -294,7 +294,11 @@ module WellFormednessArg: TypedtreeIter.IteratorArgument = { exp_desc: TExpConstant( Const_number( - (Const_number_int(_) | Const_number_float(_)) as n, + ( + Const_number_int(_) | Const_number_float(_) | + Const_number_rational(_) | + Const_number_bigint(_) + ) as n, ), ), }, @@ -311,13 +315,17 @@ module WellFormednessArg: TypedtreeIter.IteratorArgument = { || modname == "Uint32" || modname == "Uint64" || modname == "Float32" - || modname == "Float64" => + || modname == "Float64" + || modname == "Rational" + || modname == "BigInt" => // NOTE: Due to type-checking, we shouldn't need to worry about ending up with a FloatXX value and a Const_number_float let n_str = switch (n) { | Const_number_int(nint) => Int64.to_string(nint) | Const_number_float(nfloat) => Float.to_string(nfloat) - | _ => failwith("Impossible") + | Const_number_rational({rational_num_rep, rational_den_rep}) => + Printf.sprintf("%s/%s", rational_num_rep, rational_den_rep) + | Const_number_bigint({bigint_rep}) => bigint_rep }; let mod_type = switch (modname) { @@ -331,9 +339,12 @@ module WellFormednessArg: TypedtreeIter.IteratorArgument = { | "Uint64" => Grain_utils.Warnings.Uint64 | "Float32" => Grain_utils.Warnings.Float32 | "Float64" => Grain_utils.Warnings.Float64 + | "Rational" => Grain_utils.Warnings.Rational + | "BigInt" => Grain_utils.Warnings.BigInt | _ => failwith("Impossible") }; - let warning = Grain_utils.Warnings.FromNumberLiteral(mod_type, n_str); + let warning = + Grain_utils.Warnings.FromNumberLiteral(mod_type, modname, n_str); if (Grain_utils.Warnings.is_active(warning)) { Grain_parsing.Location.prerr_warning(exp_loc, warning); }; diff --git a/compiler/src/utils/warnings.re b/compiler/src/utils/warnings.re index a641b9741f..4d7d970442 100644 --- a/compiler/src/utils/warnings.re +++ b/compiler/src/utils/warnings.re @@ -16,7 +16,9 @@ type number_type = | Uint32 | Uint64 | Float32 - | Float64; + | Float64 + | Rational + | BigInt; type t = | LetRecNonFunction(string) @@ -36,7 +38,7 @@ type t = | ShadowConstructor(string) | NoCmiFile(string, option(string)) | FuncWasmUnsafe(string, string, string) - | FromNumberLiteral(number_type, string) + | FromNumberLiteral(number_type, string, string) | UselessRecordSpread; let last_warning_number = 19; @@ -60,7 +62,7 @@ let number = | NonClosedRecordPattern(_) => 15 | UnusedExtension => 16 | FuncWasmUnsafe(_, _, _) => 17 - | FromNumberLiteral(_, _) => 18 + | FromNumberLiteral(_, _, _) => 18 | UselessRecordSpread => last_warning_number; let message = @@ -129,25 +131,25 @@ let message = ++ " from the `" ++ m ++ "` module the instead." - | FromNumberLiteral(mod_type, n) => { - let (mod_name, literal) = + | FromNumberLiteral(mod_type, mod_name, n) => { + let literal = switch (mod_type) { - | Int8 => ("Int8", Printf.sprintf("%ss", n)) - | Int16 => ("Int16", Printf.sprintf("%sS", n)) - | Int32 => ("Int32", Printf.sprintf("%sl", n)) - | Int64 => ("Int64", Printf.sprintf("%sL", n)) - | Uint8 => ("Uint8", Printf.sprintf("%sus", n)) - | Uint16 => ("Uint16", Printf.sprintf("%suS", n)) - | Uint32 => ("Uint32", Printf.sprintf("%sul", n)) - | Uint64 => ("Uint64", Printf.sprintf("%suL", n)) - | Float32 => ( - "Float32", - Printf.sprintf("%sf", String.contains(n, '.') ? n : n ++ "."), - ) - | Float64 => ( - "Float64", - Printf.sprintf("%sd", String.contains(n, '.') ? n : n ++ "."), - ) + | Int8 => Printf.sprintf("%ss", n) + | Int16 => Printf.sprintf("%sS", n) + | Int32 => Printf.sprintf("%sl", n) + | Int64 => Printf.sprintf("%sL", n) + | Uint8 => Printf.sprintf("%sus", n) + | Uint16 => Printf.sprintf("%suS", n) + | Uint32 => Printf.sprintf("%sul", n) + | Uint64 => Printf.sprintf("%suL", n) + | Float32 => + Printf.sprintf("%sf", String.contains(n, '.') ? n : n ++ ".") + + | Float64 => + Printf.sprintf("%sd", String.contains(n, '.') ? n : n ++ ".") + | Rational => + Printf.sprintf("%sr", String.contains(n, '/') ? n : n ++ "/1") + | BigInt => Printf.sprintf("%st", n) }; Printf.sprintf( "it looks like you are calling %s.fromNumber() with a constant number. Try using the literal syntax (e.g. `%s`) instead.", @@ -198,7 +200,7 @@ let defaults = [ ShadowConstructor(""), NoCmiFile("", None), FuncWasmUnsafe("", "", ""), - FromNumberLiteral(Int8, ""), + FromNumberLiteral(Int8, "", ""), UselessRecordSpread, ]; diff --git a/compiler/src/utils/warnings.rei b/compiler/src/utils/warnings.rei index 667d962085..de92c12121 100644 --- a/compiler/src/utils/warnings.rei +++ b/compiler/src/utils/warnings.rei @@ -30,7 +30,9 @@ type number_type = | Uint32 | Uint64 | Float32 - | Float64; + | Float64 + | Rational + | BigInt; type t = | LetRecNonFunction(string) @@ -50,7 +52,7 @@ type t = | ShadowConstructor(string) | NoCmiFile(string, option(string)) | FuncWasmUnsafe(string, string, string) - | FromNumberLiteral(number_type, string) + | FromNumberLiteral(number_type, string, string) | UselessRecordSpread; let is_active: t => bool; diff --git a/compiler/test/suites/numbers.re b/compiler/test/suites/numbers.re index e678e99df2..ba1796fecd 100644 --- a/compiler/test/suites/numbers.re +++ b/compiler/test/suites/numbers.re @@ -200,35 +200,49 @@ describe("numbers", ({test, testSkip}) => { // well-formedness warnings test("float32_fromNumber_warn1", ({expect}) => { - expect.string(Warnings.message(FromNumberLiteral(Float32, "5"))).toMatch( + expect.string( + Warnings.message(FromNumberLiteral(Float32, "Float32", "5")), + ). + toMatch( "5.f", ) }); test("float32_fromNumber_warn2", ({expect}) => { - expect.string(Warnings.message(FromNumberLiteral(Float32, "5."))). + expect.string( + Warnings.message(FromNumberLiteral(Float32, "Float32", "5.")), + ). toMatch( "5.f", ) }); test("float32_fromNumber_warn3", ({expect}) => { - expect.string(Warnings.message(FromNumberLiteral(Float32, "5.5"))). + expect.string( + Warnings.message(FromNumberLiteral(Float32, "Float32", "5.5")), + ). toMatch( "5.5f", ) }); test("float64_fromNumber_warn1", ({expect}) => { - expect.string(Warnings.message(FromNumberLiteral(Float64, "5"))).toMatch( + expect.string( + Warnings.message(FromNumberLiteral(Float64, "Float64", "5")), + ). + toMatch( "5.d", ) }); test("float64_fromNumber_warn2", ({expect}) => { - expect.string(Warnings.message(FromNumberLiteral(Float64, "5."))). + expect.string( + Warnings.message(FromNumberLiteral(Float64, "Float64", "5.")), + ). toMatch( "5.d", ) }); test("float64_fromNumber_warn3", ({expect}) => { - expect.string(Warnings.message(FromNumberLiteral(Float64, "5.5"))). + expect.string( + Warnings.message(FromNumberLiteral(Float64, "Float64", "5.5")), + ). toMatch( "5.5d", ) From e386d59db6374bbb709179d9817ef32313aa910f Mon Sep 17 00:00:00 2001 From: Spotandjake Date: Sun, 25 Feb 2024 16:46:03 -0500 Subject: [PATCH 3/4] chore: Add tests --- compiler/test/suites/numbers.re | 80 ++++++++++++++++++++++++++++++++- 1 file changed, 79 insertions(+), 1 deletion(-) diff --git a/compiler/test/suites/numbers.re b/compiler/test/suites/numbers.re index ba1796fecd..fcc6974754 100644 --- a/compiler/test/suites/numbers.re +++ b/compiler/test/suites/numbers.re @@ -199,7 +199,61 @@ describe("numbers", ({test, testSkip}) => { ); // well-formedness warnings - test("float32_fromNumber_warn1", ({expect}) => { + test("short_fromNumber_warn1", ({expect}) => { + expect.string(Warnings.message(FromNumberLiteral(Uint8, "Uint8", "2"))). + toMatch( + "2us", + ) + }); + test("short_fromNumber_warn2", ({expect}) => { + expect.string( + Warnings.message(FromNumberLiteral(Uint16, "Uint16", "2")), + ). + toMatch( + "2uS", + ) + }); + test("short_fromNumber_warn3", ({expect}) => { + expect.string( + Warnings.message(FromNumberLiteral(Uint32, "Uint32", "2")), + ). + toMatch( + "2ul", + ) + }); + test("short_fromNumber_warn4", ({expect}) => { + expect.string( + Warnings.message(FromNumberLiteral(Uint64, "Uint64", "2")), + ). + toMatch( + "2uL", + ) + }); + test("short_fromNumber_warn5", ({expect}) => { + expect.string(Warnings.message(FromNumberLiteral(Int8, "Int8", "2"))). + toMatch( + "2s", + ) + }); + test("short_fromNumber_warn6", ({expect}) => { + expect.string(Warnings.message(FromNumberLiteral(Int16, "Int16", "2"))). + toMatch( + "2S", + ) + }); + test("short_fromNumber_warn7", ({expect}) => { + expect.string(Warnings.message(FromNumberLiteral(Int32, "Int32", "2"))). + toMatch( + "2l", + ) + }); + test("short_fromNumber_warn8", ({expect}) => { + expect.string(Warnings.message(FromNumberLiteral(Int64, "Int64", "2"))). + toMatch( + "2L", + ) + }); + test("float32_fromNumber_warn9", ({expect}) => { expect.string( Warnings.message(FromNumberLiteral(Float32, "Float32", "5")), ). @@ -247,4 +301,28 @@ describe("numbers", ({test, testSkip}) => { "5.5d", ) }); + test("rational_fromNumber_warn1", ({expect}) => { + expect.string( + Warnings.message(FromNumberLiteral(Rational, "Rational", "2")), + ). + toMatch( + "2/1r", + ) + }); + test("rational_fromNumber_warn2", ({expect}) => { + expect.string( + Warnings.message(FromNumberLiteral(Rational, "Rational", "2/4")), + ). + toMatch( + "2/4r", + ) + }); + test("bigint_fromNumber_warn1", ({expect}) => { + expect.string( + Warnings.message(FromNumberLiteral(BigInt, "Bigint", "2")), + ). + toMatch( + "2t", + ) + }); }); From b091ce69864f6247fbfd9a704c92c172b611a3bc Mon Sep 17 00:00:00 2001 From: Spotandjake Date: Sun, 3 Mar 2024 14:14:15 -0500 Subject: [PATCH 4/4] chore: Simplify pattern match --- compiler/src/typed/typed_well_formedness.re | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) diff --git a/compiler/src/typed/typed_well_formedness.re b/compiler/src/typed/typed_well_formedness.re index 0e0b4cf839..e8efb66628 100644 --- a/compiler/src/typed/typed_well_formedness.re +++ b/compiler/src/typed/typed_well_formedness.re @@ -287,23 +287,7 @@ module WellFormednessArg: TypedtreeIter.IteratorArgument = { ), }, _, - [ - ( - _, - { - exp_desc: - TExpConstant( - Const_number( - ( - Const_number_int(_) | Const_number_float(_) | - Const_number_rational(_) | - Const_number_bigint(_) - ) as n, - ), - ), - }, - ), - ], + [(_, {exp_desc: TExpConstant(Const_number(n))})], ) when modname == "Int8"