parser,checker,ast: support @[must_use] tag for fns/methods, and an experimental -check-result option (#22983)

This commit is contained in:
Delyan Angelov 2024-11-27 07:11:40 +02:00 committed by GitHub
parent 4897d7860e
commit 844d89fd09
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 108 additions and 4 deletions

View file

@ -5896,6 +5896,18 @@ fn C.DefWindowProc(hwnd int, msg int, lparam int, wparam int)
@[callconv: 'fastcall'] @[callconv: 'fastcall']
type FastFn = fn (int) bool 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) // 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: // 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. // If run from a console or terminal; keep the terminal open so all (e)println statements can be viewed.

View file

@ -583,6 +583,7 @@ pub:
is_exported bool // true for `@[export: 'exact_C_name']` 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_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_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_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]` is_file_translated bool // true, when the file it resides in is `@[translated]`
receiver StructField // TODO: this is not a struct field receiver StructField // TODO: this is not a struct field
@ -674,6 +675,7 @@ pub:
is_deprecated bool // `@[deprecated] fn abc(){}` is_deprecated bool // `@[deprecated] fn abc(){}`
is_noreturn bool // `@[noreturn] fn abc(){}` is_noreturn bool // `@[noreturn] fn abc(){}`
is_unsafe bool // `@[unsafe] fn abc(){}` is_unsafe bool // `@[unsafe] fn abc(){}`
is_must_use bool // `@[must_use] fn abc(){}`
is_placeholder bool is_placeholder bool
is_main bool // `fn main(){}` is_main bool // `fn main(){}`
is_test bool // `fn test_abc(){}` is_test bool // `fn test_abc(){}`

View file

@ -769,7 +769,6 @@ fn (mut c Checker) call_expr(mut node ast.CallExpr) ast.Type {
node.or_block.pos) node.or_block.pos)
} }
} }
return typ return typ
} }
@ -987,6 +986,11 @@ fn (mut c Checker) fn_call(mut node ast.CallExpr, mut continue_check &bool) ast.
mut func := ast.Fn{} mut func := ast.Fn{}
mut found := false mut found := false
mut found_in_args := false mut found_in_args := false
defer {
if found {
c.check_must_use_call_result(node, func, 'function')
}
}
// anon fn direct call // anon fn direct call
if node.left is ast.AnonFn { if node.left is ast.AnonFn {
// it was set to anon for checker errors, clear for gen // it was set to anon for checker errors, clear for gen
@ -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) c.error('unknown function: ${node.get_name()}', node.pos)
return ast.void_type return ast.void_type
} }
node.is_file_translated = func.is_file_translated node.is_file_translated = func.is_file_translated
node.is_noreturn = func.is_noreturn node.is_noreturn = func.is_noreturn
node.is_expand_simple_interpolation = func.is_expand_simple_interpolation node.is_expand_simple_interpolation = func.is_expand_simple_interpolation
@ -2238,6 +2243,11 @@ fn (mut c Checker) method_call(mut node ast.CallExpr, mut continue_check &bool)
mut method := ast.Fn{} mut method := ast.Fn{}
mut has_method := false mut has_method := false
mut is_method_from_embed := 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) { if m := c.table.find_method(left_sym, method_name) {
method = m method = m
has_method = true has_method = true
@ -3953,3 +3963,20 @@ fn (mut c Checker) resolve_return_type(node ast.CallExpr) ast.Type {
} }
return node.return_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)
}
}

View file

@ -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 | }

View file

@ -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())
}

View file

@ -54,8 +54,9 @@ NB: the build flags are shared with the run command too:
Define the provided flag. Define the provided flag.
If `value` is not provided, it is assumed to be set to `true`. 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), `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>)` `v` (char) or `"V rocks"` (string).
to retrieve the value assigned to `<flag>`. 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>`. If no flag identifier (or value) is assigned, `$d()` will return the passed `<default value>`.
-g -g
@ -245,6 +246,12 @@ NB: the build flags are shared with the run command too:
NB: in the future, this will be turned ON by default, NB: in the future, this will be turned ON by default,
and will become an error, after vlib modules are cleaned up. 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 C-specific build flags, use `v help build-c`.
For JS-specific build flags, use `v help build-js`. For JS-specific build flags, use `v help build-js`.
For Native-specific build flags, use `v help build-native`. For Native-specific build flags, use `v help build-native`.

View file

@ -194,6 +194,7 @@ fn (mut p Parser) fn_decl() ast.FnDecl {
mut is_keep_alive := false mut is_keep_alive := false
mut is_exported := false mut is_exported := false
mut is_unsafe := false mut is_unsafe := false
mut is_must_use := false
mut is_trusted := false mut is_trusted := false
mut is_noreturn := false mut is_noreturn := false
mut is_ctor_new := false mut is_ctor_new := false
@ -228,6 +229,9 @@ fn (mut p Parser) fn_decl() ast.FnDecl {
'unsafe' { 'unsafe' {
is_unsafe = true is_unsafe = true
} }
'must_use' {
is_must_use = true
}
'trusted' { 'trusted' {
is_trusted = true is_trusted = true
} }
@ -544,6 +548,7 @@ run them via `v file.v` instead',
is_deprecated: is_deprecated is_deprecated: is_deprecated
is_noreturn: is_noreturn is_noreturn: is_noreturn
is_unsafe: is_unsafe is_unsafe: is_unsafe
is_must_use: is_must_use
is_main: is_main is_main: is_main
is_test: is_test is_test: is_test
is_keep_alive: is_keep_alive is_keep_alive: is_keep_alive
@ -597,6 +602,7 @@ run them via `v file.v` instead',
is_noreturn: is_noreturn is_noreturn: is_noreturn
is_ctor_new: is_ctor_new is_ctor_new: is_ctor_new
is_unsafe: is_unsafe is_unsafe: is_unsafe
is_must_use: is_must_use
is_main: is_main is_main: is_main
is_test: is_test is_test: is_test
is_keep_alive: is_keep_alive is_keep_alive: is_keep_alive
@ -676,6 +682,7 @@ run them via `v file.v` instead',
is_test: is_test is_test: is_test
is_keep_alive: is_keep_alive is_keep_alive: is_keep_alive
is_unsafe: is_unsafe is_unsafe: is_unsafe
is_must_use: is_must_use
is_markused: is_markused is_markused: is_markused
is_file_translated: p.is_translated is_file_translated: p.is_translated
// //

View file

@ -126,6 +126,7 @@ pub mut:
is_callstack bool // turn on callstack registers on each call when v.debug is imported 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_trace bool // turn on possibility to trace fn call where v.debug is imported
is_coverage bool // turn on code coverage stats 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. 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' 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 profile_file string // the profile results will be stored inside profile_file
@ -956,6 +957,9 @@ pub fn parse_args_and_show_errors(known_external_commands []string, args []strin
'-check-unused-fn-args' { '-check-unused-fn-args' {
res.show_unused_params = true res.show_unused_params = true
} }
'-check-return' {
res.is_check_return = true
}
'-use-coroutines' { '-use-coroutines' {
res.use_coroutines = true res.use_coroutines = true
$if macos || linux { $if macos || linux {