From e6cd0175ec08386267b9071f0e1018419053ff71 Mon Sep 17 00:00:00 2001 From: Felipe Pena Date: Mon, 20 Jan 2025 08:03:10 -0300 Subject: [PATCH 1/7] implement --- cmd/tools/vast/vast.v | 1 + cmd/tools/vvet/analyze.v | 44 +++++++++++++++++++++++++++++++++------- cmd/tools/vvet/errors.v | 2 +- vlib/v/ast/ast.v | 1 + vlib/v/parser/fn.v | 1 + 5 files changed, 41 insertions(+), 8 deletions(-) diff --git a/cmd/tools/vast/vast.v b/cmd/tools/vast/vast.v index 9ad057c91ea894..3416b5efc32c8d 100644 --- a/cmd/tools/vast/vast.v +++ b/cmd/tools/vast/vast.v @@ -582,6 +582,7 @@ fn (t Tree) fn_decl(node ast.FnDecl) &Node { obj.add('is_direct_arr', t.bool_node(node.is_direct_arr)) obj.add('ctdefine_idx', t.number_node(node.ctdefine_idx)) obj.add('pos', t.pos(node.pos)) + obj.add('end_pos', t.pos(node.end_pos)) obj.add('body_pos', t.pos(node.body_pos)) obj.add('return_type_pos', t.pos(node.return_type_pos)) obj.add('file', t.string_node(node.file)) diff --git a/cmd/tools/vvet/analyze.v b/cmd/tools/vvet/analyze.v index d651118f197291..79ced8230294f9 100644 --- a/cmd/tools/vvet/analyze.v +++ b/cmd/tools/vvet/analyze.v @@ -16,6 +16,10 @@ const stringliteral_cutoff = 10 const ascast_cutoff = 10 const stringconcat_cutoff = 10 +// possibly inline fn cutoff +const fns_call_cutoff = 10 // at least N calls +const short_fns_cutoff = 3 // lines + // minimum size for string literals const stringliteral_min_size = 20 @@ -24,9 +28,11 @@ const long_fns_cutoff = 300 struct VetAnalyze { mut: - repeated_expr_cutoff shared map[string]int // repeated code cutoff - repeated_expr shared map[string]map[string]map[string][]token.Pos // repeated exprs in fn scope - cur_fn ast.FnDecl // current fn declaration + repeated_expr_cutoff shared map[string]int // repeated code cutoff + repeated_expr shared map[string]map[string]map[string][]token.Pos // repeated exprs in fn scope + potential_non_inlined shared map[string]map[string]token.Pos // fns might be inlined + call_counter shared map[string]int // fn call counter + cur_fn ast.FnDecl // current fn declaration } // stmt checks for repeated code in statements @@ -86,6 +92,9 @@ fn (mut vt VetAnalyze) expr(vet &Vet, expr ast.Expr) { vt.save_expr(callexpr_cutoff, '${expr.name}(${expr.args.map(it.str()).join(', ')})', vet.file, expr.pos) } + lock vt.call_counter { + vt.call_counter[expr.name]++ + } } ast.AsCast { vt.save_expr(ascast_cutoff, ast.Expr(expr).str(), vet.file, expr.pos) @@ -104,10 +113,14 @@ fn (mut vt VetAnalyze) expr(vet &Vet, expr ast.Expr) { // long_or_empty_fns checks for long or empty functions fn (mut vt VetAnalyze) long_or_empty_fns(mut vet Vet, fn_decl ast.FnDecl) { - nr_lines := if fn_decl.stmts.len == 0 { - 0 - } else { - fn_decl.stmts.last().pos.line_nr - fn_decl.pos.line_nr + nr_lines := fn_decl.end_pos.line_nr - fn_decl.pos.line_nr - 2 + if nr_lines < short_fns_cutoff { + attr := fn_decl.attrs.find_first('inline') + if attr == none { + lock vt.potential_non_inlined { + vt.potential_non_inlined[fn_decl.name][vet.file] = fn_decl.pos + } + } } if nr_lines > long_fns_cutoff { vet.notice('Long function - ${nr_lines} lines long.', fn_decl.pos.line_nr, .long_fns) @@ -137,9 +150,26 @@ fn (mut vt VetAnalyze) vet_repeated_code(mut vet Vet) { } } +// vet_inlining_fn reports possible fn to be inlined +fn (mut vt VetAnalyze) vet_inlining_fn(mut vet Vet) { + for fn_name, info in vt.potential_non_inlined { + for file, pos in info { + calls := vt.call_counter[fn_name] or { 0 } + if calls < fns_call_cutoff { + continue + } + vet.notice_with_file(file, '${fn_name} might be inlined (possibly called at least ${calls} times)', + pos.line_nr, .inline_fn) + } + } +} + // vet_code_analyze performs code analysis fn (mut vt Vet) vet_code_analyze() { if vt.opt.repeated_code { vt.analyze.vet_repeated_code(mut vt) } + if vt.opt.fn_sizing { + vt.analyze.vet_inlining_fn(mut vt) + } } diff --git a/cmd/tools/vvet/errors.v b/cmd/tools/vvet/errors.v index 7c9f63eacd22bd..8164e75e852fdf 100644 --- a/cmd/tools/vvet/errors.v +++ b/cmd/tools/vvet/errors.v @@ -16,6 +16,7 @@ pub enum FixKind { repeated_code long_fns empty_fn + inline_fn } // ErrorType is used to filter out false positive errors under specific conditions @@ -23,7 +24,6 @@ pub enum ErrorType { default space_indent trailing_space - long_fns } @[minify] diff --git a/vlib/v/ast/ast.v b/vlib/v/ast/ast.v index 67ff7914e4d721..340090fe5d619a 100644 --- a/vlib/v/ast/ast.v +++ b/vlib/v/ast/ast.v @@ -630,6 +630,7 @@ pub mut: scope &Scope = unsafe { nil } label_names []string pos token.Pos // function declaration position + end_pos token.Pos // end position // is_expand_simple_interpolation bool // true, when @[expand_simple_interpolation] is used on a fn. It should have a single string argument. } diff --git a/vlib/v/parser/fn.v b/vlib/v/parser/fn.v index 7df0d5372c77b0..bcc24ed3923350 100644 --- a/vlib/v/parser/fn.v +++ b/vlib/v/parser/fn.v @@ -707,6 +707,7 @@ run them via `v file.v` instead', language: language no_body: no_body pos: start_pos.extend_with_last_line(end_pos, p.prev_tok.line_nr) + end_pos: p.tok.pos() name_pos: name_pos body_pos: body_start_pos file: p.file_path From 0bf9e9c7e2939e1d1ebddc71b86f7e7229e94506 Mon Sep 17 00:00:00 2001 From: Felipe Pena Date: Mon, 20 Jan 2025 08:05:18 -0300 Subject: [PATCH 2/7] update help --- vlib/v/help/common/vet.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/vlib/v/help/common/vet.txt b/vlib/v/help/common/vet.txt index 42e17af9e0cd75..6beaf606ec54de 100644 --- a/vlib/v/help/common/vet.txt +++ b/vlib/v/help/common/vet.txt @@ -12,7 +12,8 @@ Options: -v, -verbose Enable verbose logging. - -F Report empty and long function declaration (>300 lines). + -F Report potential function to be inlined, empty, long function + declaration (>300 lines). -p Report private functions with missing documentation too (by default, only the `pub fn` functions will be reported). From d9d0989422a11d98edacf1b423ebcdc7f034c8b0 Mon Sep 17 00:00:00 2001 From: Felipe Pena Date: Mon, 20 Jan 2025 08:09:01 -0300 Subject: [PATCH 3/7] add customizable param via $d() --- cmd/tools/vvet/analyze.v | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/cmd/tools/vvet/analyze.v b/cmd/tools/vvet/analyze.v index 79ced8230294f9..efbf8a332b7cb8 100644 --- a/cmd/tools/vvet/analyze.v +++ b/cmd/tools/vvet/analyze.v @@ -7,28 +7,28 @@ import v.token import arrays // cutoffs -const indexexpr_cutoff = 10 -const infixexpr_cutoff = 10 -const selectorexpr_cutoff = 10 -const callexpr_cutoff = 10 -const stringinterliteral_cutoff = 10 -const stringliteral_cutoff = 10 -const ascast_cutoff = 10 -const stringconcat_cutoff = 10 +const indexexpr_cutoff = $d('VET_INDEXEXPR_CUTOFF', 10) +const infixexpr_cutoff = $d('VET_INFIXEXPR_CUTOFF', 10) +const selectorexpr_cutoff = $d('VET_SELECTOREXPR_CUTOFF', 10) +const callexpr_cutoff = $d('VET_CALLEXPR_CUTOFF', 10) +const stringinterliteral_cutoff = $d('STRINGINTERLITERAL_CUTOFF', 10) +const stringliteral_cutoff = $d('STRINGLITERAL_CUTOFF', 10) +const ascast_cutoff = $d('ASCAST_CUTOFF', 10) +const stringconcat_cutoff = $d('STRINGCONCAT_CUTOFF', 10) // possibly inline fn cutoff -const fns_call_cutoff = 10 // at least N calls -const short_fns_cutoff = 3 // lines +const fns_call_cutoff = $d('VET_FNS_CALL_CUTOFF', 10) // at least N calls +const short_fns_cutoff = $d('VET_SHORT_FNS_CUTOFF', 3) // lines // minimum size for string literals -const stringliteral_min_size = 20 +const stringliteral_min_size = $d('VET_STRINGLITERAL_MIN_SIZE', 20) // long functions cutoff -const long_fns_cutoff = 300 +const long_fns_cutoff = $d('VET_LONG_FNS_CUTOFF', 300) struct VetAnalyze { mut: - repeated_expr_cutoff shared map[string]int // repeated code cutoff + repeated_expr_cutoff shared map[string]i64 // repeated code cutoff repeated_expr shared map[string]map[string]map[string][]token.Pos // repeated exprs in fn scope potential_non_inlined shared map[string]map[string]token.Pos // fns might be inlined call_counter shared map[string]int // fn call counter @@ -51,7 +51,7 @@ fn (mut vt VetAnalyze) stmt(vet &Vet, stmt ast.Stmt) { } // save_expr registers a repeated code occurrence -fn (mut vt VetAnalyze) save_expr(cutoff int, expr string, file string, pos token.Pos) { +fn (mut vt VetAnalyze) save_expr(cutoff i64, expr string, file string, pos token.Pos) { lock vt.repeated_expr { vt.repeated_expr[vt.cur_fn.name][expr][file] << pos } From a424f053619838d373004ad5eb600fc43a2a8755 Mon Sep 17 00:00:00 2001 From: Felipe Pena Date: Mon, 20 Jan 2025 08:53:39 -0300 Subject: [PATCH 4/7] fix left type method call identification --- cmd/tools/vvet/analyze.v | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/cmd/tools/vvet/analyze.v b/cmd/tools/vvet/analyze.v index efbf8a332b7cb8..2b4ea14cfa23b8 100644 --- a/cmd/tools/vvet/analyze.v +++ b/cmd/tools/vvet/analyze.v @@ -86,15 +86,21 @@ fn (mut vt VetAnalyze) expr(vet &Vet, expr ast.Expr) { } ast.CallExpr { if expr.is_static_method || expr.is_method { - vt.save_expr(callexpr_cutoff, '${expr.left}.${expr.name}(${expr.args.map(it.str()).join(', ')})', + left_str := expr.left.str() + lock vt.call_counter { + if vt.cur_fn.receiver.name == left_str { + vt.call_counter['${int(vt.cur_fn.receiver.typ)}.${expr.name}']++ + } + } + vt.save_expr(callexpr_cutoff, '${left_str}.${expr.name}(${expr.args.map(it.str()).join(', ')})', vet.file, expr.pos) } else { + lock vt.call_counter { + vt.call_counter[expr.name]++ + } vt.save_expr(callexpr_cutoff, '${expr.name}(${expr.args.map(it.str()).join(', ')})', vet.file, expr.pos) } - lock vt.call_counter { - vt.call_counter[expr.name]++ - } } ast.AsCast { vt.save_expr(ascast_cutoff, ast.Expr(expr).str(), vet.file, expr.pos) @@ -118,7 +124,7 @@ fn (mut vt VetAnalyze) long_or_empty_fns(mut vet Vet, fn_decl ast.FnDecl) { attr := fn_decl.attrs.find_first('inline') if attr == none { lock vt.potential_non_inlined { - vt.potential_non_inlined[fn_decl.name][vet.file] = fn_decl.pos + vt.potential_non_inlined[fn_decl.fkey()][vet.file] = fn_decl.pos } } } @@ -158,7 +164,7 @@ fn (mut vt VetAnalyze) vet_inlining_fn(mut vet Vet) { if calls < fns_call_cutoff { continue } - vet.notice_with_file(file, '${fn_name} might be inlined (possibly called at least ${calls} times)', + vet.notice_with_file(file, '${fn_name.all_after('.')} fn might be inlined (possibly called at least ${calls} times)', pos.line_nr, .inline_fn) } } From 63985ebc4650223a7a0a1cb55282b4e6801fafb0 Mon Sep 17 00:00:00 2001 From: Felipe Pena Date: Mon, 20 Jan 2025 19:59:04 -0300 Subject: [PATCH 5/7] fix --- cmd/tools/vvet/analyze.v | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/cmd/tools/vvet/analyze.v b/cmd/tools/vvet/analyze.v index 2b4ea14cfa23b8..4759b256e100bc 100644 --- a/cmd/tools/vvet/analyze.v +++ b/cmd/tools/vvet/analyze.v @@ -4,27 +4,28 @@ module main import v.ast import v.token +import os import arrays // cutoffs -const indexexpr_cutoff = $d('VET_INDEXEXPR_CUTOFF', 10) -const infixexpr_cutoff = $d('VET_INFIXEXPR_CUTOFF', 10) -const selectorexpr_cutoff = $d('VET_SELECTOREXPR_CUTOFF', 10) -const callexpr_cutoff = $d('VET_CALLEXPR_CUTOFF', 10) -const stringinterliteral_cutoff = $d('STRINGINTERLITERAL_CUTOFF', 10) -const stringliteral_cutoff = $d('STRINGLITERAL_CUTOFF', 10) -const ascast_cutoff = $d('ASCAST_CUTOFF', 10) -const stringconcat_cutoff = $d('STRINGCONCAT_CUTOFF', 10) +const indexexpr_cutoff = os.getenv_opt('VET_INDEXEXPR_CUTOFF') or { '10' }.int() +const infixexpr_cutoff = os.getenv_opt('VET_INFIXEXPR_CUTOFF') or { '10' }.int() +const selectorexpr_cutoff = os.getenv_opt('VET_SELECTOREXPR_CUTOFF') or { '10' }.int() +const callexpr_cutoff = os.getenv_opt('VET_CALLEXPR_CUTOFF') or { '10' }.int() +const stringinterliteral_cutoff = os.getenv_opt('STRINGINTERLITERAL_CUTOFF') or { '10' }.int() +const stringliteral_cutoff = os.getenv_opt('STRINGLITERAL_CUTOFF') or { '10' }.int() +const ascast_cutoff = os.getenv_opt('ASCAST_CUTOFF') or { '10' }.int() +const stringconcat_cutoff = os.getenv_opt('STRINGCONCAT_CUTOFF') or { '10' }.int() // possibly inline fn cutoff -const fns_call_cutoff = $d('VET_FNS_CALL_CUTOFF', 10) // at least N calls -const short_fns_cutoff = $d('VET_SHORT_FNS_CUTOFF', 3) // lines +const fns_call_cutoff = os.getenv_opt('VET_FNS_CALL_CUTOFF') or { '10' }.int() // at least N calls +const short_fns_cutoff = os.getenv_opt('VET_SHORT_FNS_CUTOFF') or { '3' }.int() // lines // minimum size for string literals -const stringliteral_min_size = $d('VET_STRINGLITERAL_MIN_SIZE', 20) +const stringliteral_min_size = os.getenv_opt('VET_STRINGLITERAL_MIN_SIZE') or { '20' }.int() // long functions cutoff -const long_fns_cutoff = $d('VET_LONG_FNS_CUTOFF', 300) +const long_fns_cutoff = os.getenv_opt('VET_LONG_FNS_CUTOFF') or { '300' }.int() struct VetAnalyze { mut: From 01c6bcd9a4faebf87bc0bf0c6690791fe33065b9 Mon Sep 17 00:00:00 2001 From: Felipe Pena Date: Mon, 20 Jan 2025 19:59:53 -0300 Subject: [PATCH 6/7] fix --- cmd/tools/vvet/analyze.v | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/tools/vvet/analyze.v b/cmd/tools/vvet/analyze.v index 4759b256e100bc..973ef3aca0dc27 100644 --- a/cmd/tools/vvet/analyze.v +++ b/cmd/tools/vvet/analyze.v @@ -29,7 +29,7 @@ const long_fns_cutoff = os.getenv_opt('VET_LONG_FNS_CUTOFF') or { '300' }.int() struct VetAnalyze { mut: - repeated_expr_cutoff shared map[string]i64 // repeated code cutoff + repeated_expr_cutoff shared map[string]int // repeated code cutoff repeated_expr shared map[string]map[string]map[string][]token.Pos // repeated exprs in fn scope potential_non_inlined shared map[string]map[string]token.Pos // fns might be inlined call_counter shared map[string]int // fn call counter @@ -52,7 +52,7 @@ fn (mut vt VetAnalyze) stmt(vet &Vet, stmt ast.Stmt) { } // save_expr registers a repeated code occurrence -fn (mut vt VetAnalyze) save_expr(cutoff i64, expr string, file string, pos token.Pos) { +fn (mut vt VetAnalyze) save_expr(cutoff int, expr string, file string, pos token.Pos) { lock vt.repeated_expr { vt.repeated_expr[vt.cur_fn.name][expr][file] << pos } From 52d68578f813eccac48f9d75d2e7e183b0075945 Mon Sep 17 00:00:00 2001 From: Felipe Pena Date: Wed, 22 Jan 2025 08:39:48 -0300 Subject: [PATCH 7/7] add -I option --- cmd/tools/vvet/analyze.v | 17 +++++++++++------ cmd/tools/vvet/vvet.v | 5 +++++ vlib/v/help/common/vet.txt | 6 ++++-- 3 files changed, 20 insertions(+), 8 deletions(-) diff --git a/cmd/tools/vvet/analyze.v b/cmd/tools/vvet/analyze.v index 973ef3aca0dc27..d155c39a27549b 100644 --- a/cmd/tools/vvet/analyze.v +++ b/cmd/tools/vvet/analyze.v @@ -120,6 +120,16 @@ fn (mut vt VetAnalyze) expr(vet &Vet, expr ast.Expr) { // long_or_empty_fns checks for long or empty functions fn (mut vt VetAnalyze) long_or_empty_fns(mut vet Vet, fn_decl ast.FnDecl) { + nr_lines := fn_decl.end_pos.line_nr - fn_decl.pos.line_nr - 2 + if nr_lines > long_fns_cutoff { + vet.notice('Long function - ${nr_lines} lines long.', fn_decl.pos.line_nr, .long_fns) + } else if nr_lines == 0 { + vet.notice('Empty function.', fn_decl.pos.line_nr, .empty_fn) + } +} + +// potential_non_inlined checks for potential fns to be inlined +fn (mut vt VetAnalyze) potential_non_inlined(mut vet Vet, fn_decl ast.FnDecl) { nr_lines := fn_decl.end_pos.line_nr - fn_decl.pos.line_nr - 2 if nr_lines < short_fns_cutoff { attr := fn_decl.attrs.find_first('inline') @@ -129,11 +139,6 @@ fn (mut vt VetAnalyze) long_or_empty_fns(mut vet Vet, fn_decl ast.FnDecl) { } } } - if nr_lines > long_fns_cutoff { - vet.notice('Long function - ${nr_lines} lines long.', fn_decl.pos.line_nr, .long_fns) - } else if nr_lines == 0 { - vet.notice('Empty function.', fn_decl.pos.line_nr, .empty_fn) - } } // vet_fn_analysis reports repeated code by scope @@ -176,7 +181,7 @@ fn (mut vt Vet) vet_code_analyze() { if vt.opt.repeated_code { vt.analyze.vet_repeated_code(mut vt) } - if vt.opt.fn_sizing { + if vt.opt.fn_inlining { vt.analyze.vet_inlining_fn(mut vt) } } diff --git a/cmd/tools/vvet/vvet.v b/cmd/tools/vvet/vvet.v index 64f1ed25529db4..168e9923167d5a 100644 --- a/cmd/tools/vvet/vvet.v +++ b/cmd/tools/vvet/vvet.v @@ -32,6 +32,7 @@ struct Options { doc_private_fns_too bool fn_sizing bool repeated_code bool + fn_inlining bool mut: is_vfmt_off bool } @@ -52,6 +53,7 @@ fn main() { || (term_colors && '-nocolor' !in vet_options) repeated_code: '-r' in vet_options fn_sizing: '-F' in vet_options + fn_inlining: '-I' in vet_options } } mut paths := cmdline.only_non_options(vet_options) @@ -296,6 +298,9 @@ fn (mut vt Vet) stmt(stmt ast.Stmt) { if vt.opt.fn_sizing { vt.analyze.long_or_empty_fns(mut vt, stmt) } + if vt.opt.fn_inlining { + vt.analyze.potential_non_inlined(mut vt, stmt) + } vt.analyze.cur_fn = old_fn_decl } ast.StructDecl { diff --git a/vlib/v/help/common/vet.txt b/vlib/v/help/common/vet.txt index 6beaf606ec54de..ba2a71013fd8ad 100644 --- a/vlib/v/help/common/vet.txt +++ b/vlib/v/help/common/vet.txt @@ -12,8 +12,10 @@ Options: -v, -verbose Enable verbose logging. - -F Report potential function to be inlined, empty, long function - declaration (>300 lines). + -F Report empty and long function declaration + (default: >300 lines). + + -I Report potential function to be inlined. -p Report private functions with missing documentation too (by default, only the `pub fn` functions will be reported).