Skip to content

Commit

Permalink
parser,checker,ast: support @[must_use] tag for fns/methods, and an…
Browse files Browse the repository at this point in the history
… experimental `-check-result` option (vlang#22983)
  • Loading branch information
spytheman authored Nov 27, 2024
1 parent 4897d78 commit 844d89f
Show file tree
Hide file tree
Showing 8 changed files with 108 additions and 4 deletions.
12 changes: 12 additions & 0 deletions doc/docs.md
Original file line number Diff line number Diff line change
Expand Up @@ -5896,6 +5896,18 @@ fn C.DefWindowProc(hwnd int, msg int, lparam int, wparam int)
@[callconv: 'fastcall']
type FastFn = fn (int) bool
// Calls to the following function, will have to use its return value somehow.
// Ignoring it, will emit warnings.
@[must_use]
fn f() int {
return 42
}
fn g() {
// just calling `f()` here, will produce a warning
println(f()) // this is fine, because the return value was used as an argument
}
// Windows only (and obsolete; instead of it, use `-subsystem windows` when compiling)
// Without this attribute all graphical apps will have the following behavior on Windows:
// If run from a console or terminal; keep the terminal open so all (e)println statements can be viewed.
Expand Down
2 changes: 2 additions & 0 deletions vlib/v/ast/ast.v
Original file line number Diff line number Diff line change
Expand Up @@ -583,6 +583,7 @@ pub:
is_exported bool // true for `@[export: 'exact_C_name']`
is_keep_alive bool // passed memory must not be freed (by GC) before function returns
is_unsafe bool // true, when @[unsafe] is used on a fn
is_must_use bool // true, when @[must_use] is used on a fn. Calls to such functions, that ignore the return value, will cause warnings.
is_markused bool // true, when an explicit `@[markused]` tag was put on a fn; `-skip-unused` will not remove that fn
is_file_translated bool // true, when the file it resides in is `@[translated]`
receiver StructField // TODO: this is not a struct field
Expand Down Expand Up @@ -674,6 +675,7 @@ pub:
is_deprecated bool // `@[deprecated] fn abc(){}`
is_noreturn bool // `@[noreturn] fn abc(){}`
is_unsafe bool // `@[unsafe] fn abc(){}`
is_must_use bool // `@[must_use] fn abc(){}`
is_placeholder bool
is_main bool // `fn main(){}`
is_test bool // `fn test_abc(){}`
Expand Down
29 changes: 28 additions & 1 deletion vlib/v/checker/fn.v
Original file line number Diff line number Diff line change
Expand Up @@ -769,7 +769,6 @@ fn (mut c Checker) call_expr(mut node ast.CallExpr) ast.Type {
node.or_block.pos)
}
}

return typ
}

Expand Down Expand Up @@ -987,6 +986,11 @@ fn (mut c Checker) fn_call(mut node ast.CallExpr, mut continue_check &bool) ast.
mut func := ast.Fn{}
mut found := false
mut found_in_args := false
defer {
if found {
c.check_must_use_call_result(node, func, 'function')
}
}
// anon fn direct call
if node.left is ast.AnonFn {
// it was set to anon for checker errors, clear for gen
Expand Down Expand Up @@ -1332,6 +1336,7 @@ fn (mut c Checker) fn_call(mut node ast.CallExpr, mut continue_check &bool) ast.
c.error('unknown function: ${node.get_name()}', node.pos)
return ast.void_type
}

node.is_file_translated = func.is_file_translated
node.is_noreturn = func.is_noreturn
node.is_expand_simple_interpolation = func.is_expand_simple_interpolation
Expand Down Expand Up @@ -2238,6 +2243,11 @@ fn (mut c Checker) method_call(mut node ast.CallExpr, mut continue_check &bool)
mut method := ast.Fn{}
mut has_method := false
mut is_method_from_embed := false
defer {
if has_method && node.is_method {
c.check_must_use_call_result(node, method, 'method')
}
}
if m := c.table.find_method(left_sym, method_name) {
method = m
has_method = true
Expand Down Expand Up @@ -3953,3 +3963,20 @@ fn (mut c Checker) resolve_return_type(node ast.CallExpr) ast.Type {
}
return node.return_type
}

fn (mut c Checker) check_must_use_call_result(node &ast.CallExpr, f &ast.Fn, label string) {
if node.is_return_used {
return
}
if f.return_type == ast.void_type {
return
}
if f.is_must_use {
c.warn('return value must be used, ${label} `${f.name}` was tagged with `@[must_use]`',
node.pos)
return
}
if c.pref.is_check_return {
c.note('return value must be used', node.pos)
}
}
14 changes: 14 additions & 0 deletions vlib/v/checker/tests/must_use.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
vlib/v/checker/tests/must_use.vv:22:2: warning: return value must be used, function `f` was tagged with `@[must_use]`
20 |
21 | fn main() {
22 | f()
| ~~~
23 | g()
24 | h()
vlib/v/checker/tests/must_use.vv:29:4: warning: return value must be used, method `m` was tagged with `@[must_use]`
27 | println(h())
28 | a := Abc(5)
29 | a.m()
| ~~~
30 | println(a.m())
31 | }
31 changes: 31 additions & 0 deletions vlib/v/checker/tests/must_use.vv
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
type Abc = int

@[must_use]
fn (a Abc) m() int {
return 7
}

@[must_use]
fn f() int {
return 42
}

fn g() int {
return 123
}

fn h() (int, int) {
return 123, 456
}

fn main() {
f()
g()
h()
println(f())
println(g())
println(h())
a := Abc(5)
a.m()
println(a.m())
}
13 changes: 10 additions & 3 deletions vlib/v/help/build/build.txt
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,9 @@ NB: the build flags are shared with the run command too:
Define the provided flag.
If `value` is not provided, it is assumed to be set to `true`.
`value` can be any pure literal like `32` (i64),`34.98` (f64), `false` (bool),
`v` (char) or `"V rocks"` (string). In V code you can use `value := $d('<flag>', <default value>)`
to retrieve the value assigned to `<flag>`.
`v` (char) or `"V rocks"` (string).
In V code you can use `value := $d('<flag>', <default value>)` to retrieve the
value assigned to `<flag>`.
If no flag identifier (or value) is assigned, `$d()` will return the passed `<default value>`.

-g
Expand Down Expand Up @@ -245,10 +246,16 @@ NB: the build flags are shared with the run command too:
NB: in the future, this will be turned ON by default,
and will become an error, after vlib modules are cleaned up.

-check-return
Note about all calls, that ignore the return value of
the corresponding fn/method.
NB: this is still experimental, the rules for it will
change, it may be dropped completely, or it may become the default.

For C-specific build flags, use `v help build-c`.
For JS-specific build flags, use `v help build-js`.
For Native-specific build flags, use `v help build-native`.
For WebAssembly-specific build flags, use `v help build-wasm`.

See also:
`v help run` for documentation regarding `v run`.
`v help run` for documentation regarding `v run`.
7 changes: 7 additions & 0 deletions vlib/v/parser/fn.v
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ fn (mut p Parser) fn_decl() ast.FnDecl {
mut is_keep_alive := false
mut is_exported := false
mut is_unsafe := false
mut is_must_use := false
mut is_trusted := false
mut is_noreturn := false
mut is_ctor_new := false
Expand Down Expand Up @@ -228,6 +229,9 @@ fn (mut p Parser) fn_decl() ast.FnDecl {
'unsafe' {
is_unsafe = true
}
'must_use' {
is_must_use = true
}
'trusted' {
is_trusted = true
}
Expand Down Expand Up @@ -544,6 +548,7 @@ run them via `v file.v` instead',
is_deprecated: is_deprecated
is_noreturn: is_noreturn
is_unsafe: is_unsafe
is_must_use: is_must_use
is_main: is_main
is_test: is_test
is_keep_alive: is_keep_alive
Expand Down Expand Up @@ -597,6 +602,7 @@ run them via `v file.v` instead',
is_noreturn: is_noreturn
is_ctor_new: is_ctor_new
is_unsafe: is_unsafe
is_must_use: is_must_use
is_main: is_main
is_test: is_test
is_keep_alive: is_keep_alive
Expand Down Expand Up @@ -676,6 +682,7 @@ run them via `v file.v` instead',
is_test: is_test
is_keep_alive: is_keep_alive
is_unsafe: is_unsafe
is_must_use: is_must_use
is_markused: is_markused
is_file_translated: p.is_translated
//
Expand Down
4 changes: 4 additions & 0 deletions vlib/v/pref/pref.v
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ pub mut:
is_callstack bool // turn on callstack registers on each call when v.debug is imported
is_trace bool // turn on possibility to trace fn call where v.debug is imported
is_coverage bool // turn on code coverage stats
is_check_return bool // -check-return, will make V produce notices about *all* call expressions with unused results. NOTE: experimental!
eval_argument string // `println(2+2)` on `v -e "println(2+2)"`. Note that this source code, will be evaluated in vsh mode, so 'v -e 'println(ls(".")!)' is valid.
test_runner string // can be 'simple' (fastest, but much less detailed), 'tap', 'normal'
profile_file string // the profile results will be stored inside profile_file
Expand Down Expand Up @@ -956,6 +957,9 @@ pub fn parse_args_and_show_errors(known_external_commands []string, args []strin
'-check-unused-fn-args' {
res.show_unused_params = true
}
'-check-return' {
res.is_check_return = true
}
'-use-coroutines' {
res.use_coroutines = true
$if macos || linux {
Expand Down

0 comments on commit 844d89f

Please sign in to comment.