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

feat(compiler): Introduce warning if non-void expression value not used or ignored #2141

Open
wants to merge 2 commits 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
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
Loading