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..d155c39a27549b 100644 --- a/cmd/tools/vvet/analyze.v +++ b/cmd/tools/vvet/analyze.v @@ -4,29 +4,36 @@ module main import v.ast import v.token +import os 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 = 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 = 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 = 20 +const stringliteral_min_size = os.getenv_opt('VET_STRINGLITERAL_MIN_SIZE') or { '20' }.int() // long functions cutoff -const long_fns_cutoff = 300 +const long_fns_cutoff = os.getenv_opt('VET_LONG_FNS_CUTOFF') or { '300' }.int() 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 @@ -80,9 +87,18 @@ 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) } @@ -104,11 +120,7 @@ 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 > long_fns_cutoff { vet.notice('Long function - ${nr_lines} lines long.', fn_decl.pos.line_nr, .long_fns) } else if nr_lines == 0 { @@ -116,6 +128,19 @@ fn (mut vt VetAnalyze) long_or_empty_fns(mut vet Vet, fn_decl ast.FnDecl) { } } +// 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') + if attr == none { + lock vt.potential_non_inlined { + vt.potential_non_inlined[fn_decl.fkey()][vet.file] = fn_decl.pos + } + } + } +} + // vet_fn_analysis reports repeated code by scope fn (mut vt VetAnalyze) vet_repeated_code(mut vet Vet) { rlock vt.repeated_expr { @@ -137,9 +162,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.all_after('.')} fn 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_inlining { + 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/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/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/help/common/vet.txt b/vlib/v/help/common/vet.txt index 42e17af9e0cd75..ba2a71013fd8ad 100644 --- a/vlib/v/help/common/vet.txt +++ b/vlib/v/help/common/vet.txt @@ -12,7 +12,10 @@ Options: -v, -verbose Enable verbose logging. - -F Report empty and 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). 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