Skip to content

Commit

Permalink
chore(compiler): Add fromNumber(<literal>) warning for short types (
Browse files Browse the repository at this point in the history
#2024)

Co-authored-by: Oscar Spencer <oscar@grain-lang.org>
  • Loading branch information
spotandjake and ospencer committed Mar 4, 2024
1 parent ca75e38 commit b09ea66
Show file tree
Hide file tree
Showing 4 changed files with 192 additions and 88 deletions.
52 changes: 28 additions & 24 deletions compiler/src/typed/typed_well_formedness.re
Expand Up @@ -324,7 +324,7 @@ module WellFormednessArg: TypedtreeIter.IteratorArgument = {
};
| _ => ()
}
// Check: Warn if using Int32.fromNumber(<literal>)
// Check: Warn if using XXXX.fromNumber(<literal>)
| TExpApp(
{
exp_desc:
Expand All @@ -335,44 +335,48 @@ module WellFormednessArg: TypedtreeIter.IteratorArgument = {
),
},
_,
[
(
Unlabeled,
{
exp_desc:
TExpConstant(
Const_number(
(Const_number_int(_) | Const_number_float(_)) as n,
),
),
},
),
],
[(_, {exp_desc: TExpConstant(Const_number(n))})],
)
when
modname == "Int32"
modname == "Int8"
|| modname == "Int16"
|| modname == "Int32"
|| modname == "Int64"
|| modname == "Uint8"
|| modname == "Uint16"
|| 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 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
| "Rational" => Grain_utils.Warnings.Rational
| "BigInt" => Grain_utils.Warnings.BigInt
| _ => failwith("Impossible")
};
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);
};
Expand Down
97 changes: 46 additions & 51 deletions compiler/src/utils/warnings.re
Expand Up @@ -6,6 +6,20 @@ type loc = {
loc_ghost: bool,
};

type number_type =
| Int8
| Int16
| Int32
| Int64
| Uint8
| Uint16
| Uint32
| Uint64
| Float32
| Float64
| Rational
| BigInt;

type t =
| LetRecNonFunction(string)
| AmbiguousName(list(string), list(string), bool)
Expand All @@ -24,17 +38,12 @@ 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, string)
| UselessRecordSpread
| PrintUnsafe(string)
| ToStringUnsafe(string);

let last_warning_number = 26;
let last_warning_number = 21;

let number =
fun
Expand All @@ -55,14 +64,9 @@ let number =
| NonClosedRecordPattern(_) => 15
| UnusedExtension => 16
| FuncWasmUnsafe(_, _, _) => 17
| FromNumberLiteralI32(_) => 18
| FromNumberLiteralI64(_) => 19
| FromNumberLiteralU32(_) => 20
| FromNumberLiteralU64(_) => 21
| FromNumberLiteralF32(_) => 22
| FromNumberLiteralF64(_) => 23
| UselessRecordSpread => 24
| PrintUnsafe(_) => 25
| FromNumberLiteral(_, _, _) => 18
| UselessRecordSpread => 19
| PrintUnsafe(_) => 20
| ToStringUnsafe(_) => last_warning_number;

let message =
Expand Down Expand Up @@ -131,36 +135,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, mod_name, n) => {
let literal =
switch (mod_type) {
| 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.",
mod_name,
literal,
);
}
| UselessRecordSpread => "this record spread is useless as all of the record's fields are overridden."
| PrintUnsafe(typ) =>
"it looks like you are using `print` on an unsafe Wasm value here.\nThis is generally unsafe and will cause errors. Use `DebugPrint.print`"
Expand Down Expand Up @@ -212,12 +212,7 @@ let defaults = [
ShadowConstructor(""),
NoCmiFile("", None),
FuncWasmUnsafe("", "", ""),
FromNumberLiteralI32(""),
FromNumberLiteralI64(""),
FromNumberLiteralU32(""),
FromNumberLiteralU64(""),
FromNumberLiteralF32(""),
FromNumberLiteralF64(""),
FromNumberLiteral(Int8, "", ""),
UselessRecordSpread,
PrintUnsafe(""),
ToStringUnsafe(""),
Expand Down
21 changes: 15 additions & 6 deletions compiler/src/utils/warnings.rei
Expand Up @@ -20,6 +20,20 @@ type loc = {
loc_ghost: bool,
};

type number_type =
| Int8
| Int16
| Int32
| Int64
| Uint8
| Uint16
| Uint32
| Uint64
| Float32
| Float64
| Rational
| BigInt;

type t =
| LetRecNonFunction(string)
| AmbiguousName(list(string), list(string), bool)
Expand All @@ -38,12 +52,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, string)
| UselessRecordSpread
| PrintUnsafe(string)
| ToStringUnsafe(string);
Expand Down
110 changes: 103 additions & 7 deletions compiler/test/suites/numbers.re
Expand Up @@ -199,34 +199,130 @@ describe("numbers", ({test, testSkip}) => {
);

// well-formedness warnings
test("float32_fromNumber_warn1", ({expect}) => {
expect.string(Warnings.message(FromNumberLiteralF32("5"))).toMatch(
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")),
).
toMatch(
"5.f",
)
});
test("float32_fromNumber_warn2", ({expect}) => {
expect.string(Warnings.message(FromNumberLiteralF32("5."))).toMatch(
expect.string(
Warnings.message(FromNumberLiteral(Float32, "Float32", "5.")),
).
toMatch(
"5.f",
)
});
test("float32_fromNumber_warn3", ({expect}) => {
expect.string(Warnings.message(FromNumberLiteralF32("5.5"))).toMatch(
expect.string(
Warnings.message(FromNumberLiteral(Float32, "Float32", "5.5")),
).
toMatch(
"5.5f",
)
});
test("float64_fromNumber_warn1", ({expect}) => {
expect.string(Warnings.message(FromNumberLiteralF64("5"))).toMatch(
expect.string(
Warnings.message(FromNumberLiteral(Float64, "Float64", "5")),
).
toMatch(
"5.d",
)
});
test("float64_fromNumber_warn2", ({expect}) => {
expect.string(Warnings.message(FromNumberLiteralF64("5."))).toMatch(
expect.string(
Warnings.message(FromNumberLiteral(Float64, "Float64", "5.")),
).
toMatch(
"5.d",
)
});
test("float64_fromNumber_warn3", ({expect}) => {
expect.string(Warnings.message(FromNumberLiteralF64("5.5"))).toMatch(
expect.string(
Warnings.message(FromNumberLiteral(Float64, "Float64", "5.5")),
).
toMatch(
"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",
)
});
});

0 comments on commit b09ea66

Please sign in to comment.