diff --git a/doc/docs.md b/doc/docs.md index 0b3d3110e9..773fb52777 100644 --- a/doc/docs.md +++ b/doc/docs.md @@ -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. diff --git a/vlib/v/ast/ast.v b/vlib/v/ast/ast.v index 3a0e66a333..35cf23bbe6 100644 --- a/vlib/v/ast/ast.v +++ b/vlib/v/ast/ast.v @@ -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 @@ -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(){}` diff --git a/vlib/v/checker/fn.v b/vlib/v/checker/fn.v index 57fa7b7bd6..b6464678a4 100644 --- a/vlib/v/checker/fn.v +++ b/vlib/v/checker/fn.v @@ -769,7 +769,6 @@ fn (mut c Checker) call_expr(mut node ast.CallExpr) ast.Type { node.or_block.pos) } } - 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 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 @@ -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 @@ -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 @@ -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) + } +} diff --git a/vlib/v/checker/tests/must_use.out b/vlib/v/checker/tests/must_use.out new file mode 100644 index 0000000000..644e4a2408 --- /dev/null +++ b/vlib/v/checker/tests/must_use.out @@ -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 | } diff --git a/vlib/v/checker/tests/must_use.vv b/vlib/v/checker/tests/must_use.vv new file mode 100644 index 0000000000..bc02a0ef96 --- /dev/null +++ b/vlib/v/checker/tests/must_use.vv @@ -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()) +} diff --git a/vlib/v/help/build/build.txt b/vlib/v/help/build/build.txt index 236b4d8bc2..a589b95f98 100644 --- a/vlib/v/help/build/build.txt +++ b/vlib/v/help/build/build.txt @@ -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('', )` - to retrieve the value assigned to ``. + `v` (char) or `"V rocks"` (string). + In V code you can use `value := $d('', )` to retrieve the + value assigned to ``. If no flag identifier (or value) is assigned, `$d()` will return the passed ``. -g @@ -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`. \ No newline at end of file + `v help run` for documentation regarding `v run`. diff --git a/vlib/v/parser/fn.v b/vlib/v/parser/fn.v index 4e51726511..1462919712 100644 --- a/vlib/v/parser/fn.v +++ b/vlib/v/parser/fn.v @@ -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 @@ -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 } @@ -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 @@ -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 @@ -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 // diff --git a/vlib/v/pref/pref.v b/vlib/v/pref/pref.v index bc757b2487..b91a76b85a 100644 --- a/vlib/v/pref/pref.v +++ b/vlib/v/pref/pref.v @@ -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 @@ -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 {