Skip to content

Commit

Permalink
feat(compiler): Introduce warning if non-void expression value not us…
Browse files Browse the repository at this point in the history
…ed or ignored (#2141)
  • Loading branch information
alex-snezhko authored Sep 22, 2024
1 parent f97c011 commit 43ce8f5
Show file tree
Hide file tree
Showing 33 changed files with 295 additions and 225 deletions.
5 changes: 2 additions & 3 deletions compiler/src/typed/typecore.re
Original file line number Diff line number Diff line change
Expand Up @@ -2254,11 +2254,10 @@ and type_statement_expr = (~explanation=?, ~in_function=?, env, sexp) => {
/*| Tarrow _ -> [not really applicable with our syntax]
Location.prerr_warning loc Warnings.Partial_application*/
| TTyConstr(p, _, _) when Path.same(p, Builtin_types.path_void) => ()
| TTyVar(None) => ()
/*| Tvar _ ->
add_delayed_check (fun () -> check_application_result env true exp)*/
| _ => ()
/* This isn't quite relevant to Grain mechanics
Location.prerr_warning loc Grain_utils.Warnings.StatementType */
| _ => Location.prerr_warning(loc, Grain_utils.Warnings.StatementType)
};
unify_var(env, tv, ty);
exp;
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/utils/warnings.re
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ let message =
"these field labels belong to several types: "
++ String.concat(" ", tl)
++ "\nThe first one was selected. Please disambiguate if this is wrong."
| StatementType => "this expression should have type void."
| StatementType => "this expression should have type void. Use `ignore` to ignore the result of the expression"
| NonreturningStatement => "this statement never returns (or has an unsound type)."
| AllClausesGuarded => "this pattern-matching is not exhaustive.\nAll clauses in this pattern-matching are guarded."
| PartialMatch("") => "this pattern-matching is not exhaustive."
Expand Down
4 changes: 2 additions & 2 deletions compiler/test/input/wasiPolyfill.gr
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ provide let fd_write: (WasmI32, WasmI32, WasmI32, WasmI32) => WasmI32 =
iovs_len,
nwritten,
) => {
fd_write(fd, iovs, iovs_len, nwritten)
fd_write(fd, iovs, iovs_len, nwritten)
let _ = fd_write(fd, iovs, iovs_len, nwritten)
let _ = fd_write(fd, iovs, iovs_len, nwritten)
fd_write(fd, iovs, iovs_len, nwritten)
}

Expand Down
2 changes: 2 additions & 0 deletions compiler/test/runner.re
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,7 @@ let makeSnapshotRunner =
(~config_fn=?, test, ~module_header=module_header, name, prog) => {
test(name, ({expect}) => {
Config.preserve_all_configs(() => {
Config.print_warnings := false;
ignore @@
compile(
~hook=stop_after_object_file_emitted,
Expand Down Expand Up @@ -249,6 +250,7 @@ let makeFilesizeRunner =
let makeSnapshotFileRunner = (test, ~config_fn=?, name, filename) => {
test(name, ({expect}) => {
Config.preserve_all_configs(() => {
Config.print_warnings := false;
let infile = grainfile(filename);
let outfile = wasmfile(name);
ignore @@
Expand Down
2 changes: 1 addition & 1 deletion compiler/test/stdlib/array.test.gr
Original file line number Diff line number Diff line change
Expand Up @@ -835,7 +835,7 @@ module Immutable {

// Array.rotate
let arr1 = fromList([1, 2, 3, 4, 5])
Array.rotate(0, arr1)
ignore(Array.rotate(0, arr1))
assert arr1 == fromList([1, 2, 3, 4, 5])

let arr2 = fromList([1, 2, 3, 4, 5])
Expand Down
2 changes: 1 addition & 1 deletion compiler/test/stdlib/char.test.gr
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ while (!done) {
break
}

Char.code(chars[charPosition])
ignore(Char.code(chars[charPosition]))
charPosition += 1
}

Expand Down
4 changes: 2 additions & 2 deletions compiler/test/stdlib/path.test.gr
Original file line number Diff line number Diff line change
Expand Up @@ -216,8 +216,8 @@ List.forEach(({ base, toAppend, final }) => {
assert append == Ok(expPath)
}, appendTests)

Path.append(fs("file"), fs("f")) == Err(Path.AppendToFile)
Path.append(fs("/d/"), fs("/f")) == Err(Path.AppendAbsolute)
assert Path.append(fs("file"), fs("f")) == Err(Path.AppendToFile)
assert Path.append(fs("/d/"), fs("/f")) == Err(Path.AppendAbsolute)

record RelativeToDirTestData {
source: String,
Expand Down
32 changes: 16 additions & 16 deletions compiler/test/stdlib/queue.test.gr
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ Queue.push(4, queue)
assert Queue.size(queue) == 3
assert Queue.peek(queue) == Some(2)
let copy = Queue.copy(queue)
Queue.pop(copy)
ignore(Queue.pop(copy))
assert Queue.size(copy) == 2
assert Queue.size(queue) == 3
Queue.clear(queue)
Expand Down Expand Up @@ -61,11 +61,11 @@ Queue.push(1, queue)
Queue.push(2, queue)
Queue.push(3, queue)
assert Queue.toList(queue) == [0, 1, 2, 3]
Queue.pop(queue)
ignore(Queue.pop(queue))
assert Queue.toList(queue) == [1, 2, 3]
Queue.push(3, queue)
assert Queue.toList(queue) == [1, 2, 3, 3]
Queue.pop(queue)
ignore(Queue.pop(queue))
Queue.push(4, queue)
Queue.push(5, queue)
assert Queue.toList(queue) == [2, 3, 3, 4, 5]
Expand Down Expand Up @@ -121,12 +121,12 @@ Queue.push(1, queue2)
Queue.push(2, queue2)
Queue.push(3, queue2)
Queue.push(4, queue2)
Queue.pop(queue2)
Queue.pop(queue2)
ignore(Queue.pop(queue2))
ignore(Queue.pop(queue2))
Queue.push(5, queue2)
Queue.pop(queue2)
Queue.pop(queue2)
Queue.pop(queue2)
ignore(Queue.pop(queue2))
ignore(Queue.pop(queue2))
ignore(Queue.pop(queue2))
Queue.push(6, queue2)
Queue.push(7, queue2)
Queue.push(8, queue2)
Expand All @@ -149,7 +149,7 @@ Queue.push(2, queue3)
Queue.push(3, queue3)
assert Queue.toArray(queue3) == [> 1, 2, 3]
let queue4 = Queue.copy(queue3)
Queue.pop(queue4)
ignore(Queue.pop(queue4))
assert Queue.toArray(queue4) == [> 2, 3]

// Queue.fromArray
Expand All @@ -169,12 +169,12 @@ Queue.push(1, queue2)
Queue.push(2, queue2)
Queue.push(3, queue2)
Queue.push(4, queue2)
Queue.pop(queue2)
Queue.pop(queue2)
ignore(Queue.pop(queue2))
ignore(Queue.pop(queue2))
Queue.push(5, queue2)
Queue.pop(queue2)
Queue.pop(queue2)
Queue.pop(queue2)
ignore(Queue.pop(queue2))
ignore(Queue.pop(queue2))
ignore(Queue.pop(queue2))
Queue.push(6, queue2)
Queue.push(7, queue2)
Queue.push(8, queue2)
Expand All @@ -194,7 +194,7 @@ Queue.push(1, queue3)
Queue.push(2, queue3)
Queue.push(3, queue3)
let queue4 = Queue.copy(queue3)
Queue.pop(queue4)
ignore(Queue.pop(queue4))
let queue5 = Queue.make()
Queue.push(6, queue5)
Queue.push(7, queue5)
Expand Down Expand Up @@ -222,7 +222,7 @@ Queue.push(3, queue7)
assert queue4 == queue7
let queue8 = Queue.make()
Queue.push(1, queue8)
Queue.pop(queue8)
ignore(Queue.pop(queue8))
assert queue8 == queue1

assert !(queue2 == queue3)
Expand Down
4 changes: 2 additions & 2 deletions compiler/test/stdlib/set.test.gr
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,7 @@ module Immutable {

let mut filterTestSet = makeTestSet()

Set.filter(key => fail "Shouldn't be called", Set.empty)
ignore(Set.filter(key => fail "Shouldn't be called", Set.empty))
filterTestSet = Set.filter(key => key == Sheep, filterTestSet)

assert !Set.contains(Grain, filterTestSet)
Expand All @@ -338,7 +338,7 @@ module Immutable {

let mut rejectTestSet = makeTestSet()

Set.reject(key => fail "Shouldn't be called", Set.empty)
ignore(Set.reject(key => fail "Shouldn't be called", Set.empty))
rejectTestSet = Set.reject(key => key == Sheep, rejectTestSet)

assert Set.contains(Grain, rejectTestSet)
Expand Down
2 changes: 1 addition & 1 deletion compiler/test/stdlib/stack.test.gr
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ Stack.push(4, stack)
assert Stack.size(stack) == 3
assert Stack.peek(stack) == Some(4)
let copy = Stack.copy(stack)
Stack.pop(copy)
ignore(Stack.pop(copy))
assert Stack.size(copy) == 2
assert Stack.size(stack) == 3
Stack.clear(stack)
Expand Down
14 changes: 7 additions & 7 deletions compiler/test/stdlib/wasi.file.test.gr
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ let foo = Result.unwrap(

let (buf, nread) = Result.unwrap(Fs.fdRead(foo, 40))

Fs.fdClose(foo)
ignore(Fs.fdClose(foo))

assert buf == Bytes.fromString("foo, bar, & baz")
assert nread == 15
Expand All @@ -23,7 +23,7 @@ let foo = Result.unwrap(

let (buf, nread) = Result.unwrap(Fs.fdRead(foo, 40))

Fs.fdClose(foo)
ignore(Fs.fdClose(foo))

assert buf == Bytes.fromString("foo, bar, & baz")
assert nread == 15
Expand All @@ -35,7 +35,7 @@ let foo = Result.unwrap(

let (buf, nread) = Result.unwrap(Fs.fdRead(foo, 40))

Fs.fdClose(foo)
ignore(Fs.fdClose(foo))

assert buf == Bytes.fromString("foo, bar, & baz")
assert nread == 15
Expand All @@ -53,7 +53,7 @@ let foo = Result.unwrap(

let (buf, nread) = Result.unwrap(Fs.fdRead(foo, 40))

Fs.fdClose(foo)
ignore(Fs.fdClose(foo))

assert buf == Bytes.fromString("foo, bar, & baz")
assert nread == 15
Expand All @@ -71,13 +71,13 @@ let foo = Result.unwrap(

assert Fs.fdWrite(foo, Bytes.fromString("this and that")) == Ok(13)

Fs.fdSeek(foo, 0L, Fs.Set)
ignore(Fs.fdSeek(foo, 0L, Fs.Set))

let (buf, nread) = Result.unwrap(Fs.fdRead(foo, 40))

Fs.fdSetSize(foo, 0L)
ignore(Fs.fdSetSize(foo, 0L))

Fs.fdClose(foo)
ignore(Fs.fdClose(foo))

assert buf == Bytes.fromString("this and that")
assert nread == 13
2 changes: 1 addition & 1 deletion compiler/test/stdlib/wasi.process.test.gr
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,4 @@ match (Process.env()) {
Err(err) => throw err,
}

Process.exit(5)
let _ = Process.exit(5)
16 changes: 8 additions & 8 deletions compiler/test/suites/arrays.re
Original file line number Diff line number Diff line change
Expand Up @@ -30,42 +30,42 @@ describe("arrays", ({test, testSkip}) => {
assertSnapshot("array_access5", "[> 1, 2, 3][-3]");
assertRunError(
"array_access_err",
"let x = [> 1, 2, 3]; x[3]",
"let x = [> 1, 2, 3]; ignore(x[3])",
"Index out of bounds",
);
assertRunError(
"array_access_err2",
"let x = [> 1, 2, 3]; x[-4]",
"let x = [> 1, 2, 3]; ignore(x[-4])",
"Index out of bounds",
);
assertRunError(
"array_access_err3",
"let x = [> 1, 2, 3]; x[99]",
"let x = [> 1, 2, 3]; ignore(x[99])",
"Index out of bounds",
);
assertRunError(
"array_access_err4",
"let x = [> 1, 2, 3]; x[-99]",
"let x = [> 1, 2, 3]; ignore(x[-99])",
"Index out of bounds",
);
assertRunError(
"array_access_err5",
"let x = [> 1, 2, 3]; let i = 1.5; x[i]",
"let x = [> 1, 2, 3]; let i = 1.5; ignore(x[i])",
"Index not an integer",
);
assertRunError(
"array_access_err6",
"let x = [> 1, 2, 3]; let i = 1/3; x[i]",
"let x = [> 1, 2, 3]; let i = 1/3; ignore(x[i])",
"Index not an integer",
);
assertRunError(
"array_access_err7",
"let x = [> 1, 2, 3]; x[987654321987654321]",
"let x = [> 1, 2, 3]; ignore(x[987654321987654321])",
"Index out of bounds",
);
assertCompileError(
"array_access_err8",
"let x = [> 1, 2, 3]; x[false]",
"let x = [> 1, 2, 3]; ignore(x[false])",
"has type Bool but",
);
assertRun(
Expand Down
6 changes: 3 additions & 3 deletions compiler/test/suites/basic_functionality.re
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,10 @@ describe("basic functionality", ({test, testSkip}) => {
assertSnapshot("binop6", "9 % 5");
assertRunError(
"division_by_zero",
"let nine = 9; nine / 0",
"let nine = 9; ignore(nine / 0)",
"Division by zero",
);
assertRunError("modulo_by_zero", "9 % 0", "Modulo by zero");
assertRunError("modulo_by_zero", "ignore(9 % 0)", "Modulo by zero");
assertSnapshot("division1", "5 / 2");
assertSnapshot("modulo1", "-17 % 4");
assertSnapshot("modulo2", "17 % -4");
Expand Down Expand Up @@ -240,7 +240,7 @@ describe("basic functionality", ({test, testSkip}) => {
assertRunError("fail1", "ignore(fail \"boo\")", "Failure: boo");
assertRunError(
"fail2",
"if (false) { 3 } else { fail \"boo\" }",
"ignore(if (false) { 3 } else { fail \"boo\" })",
"Failure: boo",
);
assertSnapshotFile("toplevel_statements", "toplevelStatements");
Expand Down
60 changes: 60 additions & 0 deletions compiler/test/suites/expressions.re
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
open Grain_tests.TestFramework;
open Grain_tests.Runner;

let {describe} =
describeConfig |> withCustomMatchers(customMatchers) |> build;

describe("expressions", ({test, testSkip}) => {
let assertNoWarning = makeNoWarningRunner(test);
let assertWarning = makeWarningRunner(test);

assertWarning(
"non_void_non_returning_expression1",
{|
1
print(2)
|},
Grain_utils.Warnings.StatementType,
);

assertWarning(
"non_void_non_returning_expression2",
{|
let f = () => {
1
print(2)
}
|},
Grain_utils.Warnings.StatementType,
);

assertNoWarning(
"non_void_non_returning_expression3",
{|
ignore(1)
print(2)
|},
);

assertNoWarning(
"non_void_non_returning_expression4",
{|
let f = () => {
ignore(1)
print(2)
}
|},
);

assertNoWarning(
"non_void_non_returning_expression5",
{|
let rec f = () => {
if (true) {
f()
void
}
}
|},
);
});
Loading

0 comments on commit 43ce8f5

Please sign in to comment.