feat(compiler): Add warning for calls to IntXX.fromNumber and FloatXX…

….fromNumber with literal integers/floats (#1218)
grain-lang · May 24, 2022 · 2fb86e5 · 2fb86e5
1 parent 37b63c4
commit 2fb86e5
Show file tree

Hide file tree

Showing 3 changed files with 91 additions and 7 deletions.
diff --git a/compiler/src/typed/typed_well_formedness.re b/compiler/src/typed/typed_well_formedness.re
@@ -83,7 +83,7 @@ module WellFormednessArg: TypedtreeIter.IteratorArgument = {
 
   let enter_expression: expression => unit =
     ({exp_desc, exp_loc, exp_attributes} as exp) => {
-      // Check #1: Avoid using Pervasives equality ops with WasmXX types
+      // Check: Avoid using Pervasives equality ops with WasmXX types
       switch (exp_desc) {
       | TExpLet(_) when is_marked_unsafe(exp_attributes) => push_unsafe(true)
       | TExpApp(
@@ -107,9 +107,57 @@ module WellFormednessArg: TypedtreeIter.IteratorArgument = {
             Grain_parsing.Location.prerr_warning(exp_loc, warning);
           };
         }
+      // Check: Warn if using Int32.fromNumber(<literal>)
+      | TExpApp(
+          {
+            exp_desc:
+              TExpIdent(
+                Path.PExternal(
+                  Path.PIdent({name: modname}),
+                  "fromNumber",
+                  _,
+                ),
+                _,
+                _,
+              ),
+          },
+          [
+            {
+              exp_desc:
+                TExpConstant(
+                  Const_number(
+                    (Const_number_int(_) | Const_number_float(_)) as n,
+                  ),
+                ),
+            },
+          ],
+        )
+          when
+            modname == "Int32"
+            || modname == "Int64"
+            || modname == "Float32"
+            || modname == "Float64" =>
+        // 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")
+          };
+        let warning =
+          switch (modname) {
+          | "Int32" => Grain_utils.Warnings.FromNumberLiteralI32(n_str)
+          | "Int64" => Grain_utils.Warnings.FromNumberLiteralI64(n_str)
+          | "Float32" => Grain_utils.Warnings.FromNumberLiteralF32(n_str)
+          | "Float64" => Grain_utils.Warnings.FromNumberLiteralF64(n_str)
+          | _ => failwith("Impossible")
+          };
+        if (Grain_utils.Warnings.is_active(warning)) {
+          Grain_parsing.Location.prerr_warning(exp_loc, warning);
+        };
       | _ => ()
       };
-      // Check #2: Forbid usage of WasmXX types outside of disableGC context
+      // Check: Forbid usage of WasmXX types outside of disableGC context
       switch (exp_desc) {
       | TExpLambda(_) => push_in_lambda(true)
       | _ => ()

diff --git a/compiler/src/utils/warnings.re b/compiler/src/utils/warnings.re
@@ -23,7 +23,11 @@ type t =
   | UnreachableCase
   | ShadowConstructor(string)
   | NoCmiFile(string, option(string))
-  | FuncWasmUnsafe(string);
+  | FuncWasmUnsafe(string)
+  | FromNumberLiteralI32(string)
+  | FromNumberLiteralI64(string)
+  | FromNumberLiteralF32(string)
+  | FromNumberLiteralF64(string);
 
 let number =
   fun
@@ -43,9 +47,13 @@ let number =
   | NoCmiFile(_) => 14
   | NonClosedRecordPattern(_) => 15
   | UnusedExtension => 16
-  | FuncWasmUnsafe(_) => 17;
+  | FuncWasmUnsafe(_) => 17
+  | FromNumberLiteralI32(_) => 18
+  | FromNumberLiteralI64(_) => 19
+  | FromNumberLiteralF32(_) => 20
+  | FromNumberLiteralF64(_) => 21;
 
-let last_warning_number = 17;
+let last_warning_number = 21;
 
 let message =
   fun
@@ -108,7 +116,27 @@ let message =
   | FuncWasmUnsafe(func) =>
     "it looks like you are using "
     ++ func
-    ++ " on two unsafe Wasm values here.\nThis is generally unsafe and will cause errors. Use one of the equivalent functions in `WasmI32`, `WasmI64`, `WasmF32`, or `WasmF64` instead.";
+    ++ " on two unsafe Wasm values here.\nThis is generally unsafe and will cause errors. Use one of the equivalent functions in `WasmI32`, `WasmI64`, `WasmF32`, or `WasmF64` 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,
+    )
+  | 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.",
+      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.",
+      n,
+    );
 
 let sub_locs =
   fun
@@ -151,6 +179,10 @@ let defaults = [
   ShadowConstructor(""),
   NoCmiFile("", None),
   FuncWasmUnsafe(""),
+  FromNumberLiteralI32(""),
+  FromNumberLiteralI64(""),
+  FromNumberLiteralF32(""),
+  FromNumberLiteralF64(""),
 ];
 
 let _ = List.iter(x => current^.active[number(x)] = true, defaults);

diff --git a/compiler/src/utils/warnings.rei b/compiler/src/utils/warnings.rei
@@ -37,7 +37,11 @@ type t =
   | UnreachableCase
   | ShadowConstructor(string)
   | NoCmiFile(string, option(string))
-  | FuncWasmUnsafe(string);
+  | FuncWasmUnsafe(string)
+  | FromNumberLiteralI32(string)
+  | FromNumberLiteralI64(string)
+  | FromNumberLiteralF32(string)
+  | FromNumberLiteralF64(string);
 
 let is_active: t => bool;
 let is_error: t => bool;