Skip to content

Commit

Permalink
vet, parser: rewrite vet error handling (improve parser performance e…
Browse files Browse the repository at this point in the history
…xtend vvet) p1 (#21417)
  • Loading branch information
ttytm committed May 4, 2024
1 parent f0abc45 commit 387af74
Show file tree
Hide file tree
Showing 4 changed files with 121 additions and 53 deletions.
8 changes: 4 additions & 4 deletions cmd/tools/install_binaryen.vsh
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,10 @@ fn main() {
// TODO: add retries here, github requests can fail
jq := http.get_text('https://api.github.com/repos/WebAssembly/binaryen/releases/latest')
tag := json.decode(JQ, jq)!.tag_name
if github_job != '' {
dump(jq)
dump(tag)
}
if github_job != '' {
dump(jq)
dump(tag)
}
*/
tag := 'version_112'

Expand Down
27 changes: 27 additions & 0 deletions cmd/tools/vvet/filter.v
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
module main

import v.token
import v.vet

type FilteredLines = map[vet.ErrorType]map[int]bool

fn (mut fl FilteredLines) comments(is_multi bool, pos token.Pos) {
if !is_multi {
return
}
for ln in pos.line_nr + 1 .. pos.last_line + 1 {
fl[.space_indent][ln] = true
}
}

fn (mut fl FilteredLines) assigns(pos token.Pos) {
if pos.line_nr == pos.last_line {
return
}
for ln in pos.line_nr + 1 .. pos.last_line {
fl[.trailing_space][ln] = true
fl[.space_indent][ln] = true
}
fl[.trailing_space][pos.line_nr] = true
fl[.space_indent][pos.last_line] = true
}
99 changes: 90 additions & 9 deletions cmd/tools/vvet/vvet.v
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,13 @@ import v.help
import term

struct Vet {
opt Options
mut:
errors []vet.Error
warns []vet.Error
notices []vet.Error
file string
opt Options
errors []vet.Error
warns []vet.Error
notices []vet.Error
file string
filtered_lines FilteredLines
}

struct Options {
Expand All @@ -28,6 +29,8 @@ struct Options {
show_warnings bool
use_color bool
doc_private_fns_too bool
mut:
is_vfmt_off bool
}

const term_colors = term.can_show_color_on_stderr()
Expand Down Expand Up @@ -107,20 +110,42 @@ fn (mut vt Vet) vet_file(path string) {
prefs.is_vsh = path.ends_with('.vsh')
mut table := ast.new_table()
vt.vprintln("vetting file '${path}'...")
_, errors, notices := parser.parse_vet_file(path, mut table, prefs)
file, errors, notices := parser.parse_vet_file(path, mut table, prefs)
vt.stmts(file.stmts)
// Transfer errors from scanner and parser
vt.errors << errors
vt.notices << notices
// Scan each line in file for things to improve
source_lines := os.read_lines(vt.file) or { []string{} }
for lnumber, line in source_lines {
vt.vet_line(source_lines, line, lnumber)
for ln, line in source_lines {
vt.vet_line(source_lines, line, ln)
}
}

// vet_line vets the contents of `line` from `vet.file`.
fn (mut vt Vet) vet_line(lines []string, line string, lnumber int) {
vt.vet_fn_documentation(lines, line, lnumber)
vt.vet_space_usage(line, lnumber)
}

fn (mut vt Vet) vet_space_usage(line string, lnumber int) {
if line.starts_with('// vfmt off') {
vt.opt.is_vfmt_off = true
} else if line.starts_with('// vfmt on') {
vt.opt.is_vfmt_off = false
}
if vt.opt.is_vfmt_off {
return
}
if lnumber !in vt.filtered_lines[.space_indent] {
if line.starts_with(' ') {
vt.error('Looks like you are using spaces for indentation.', lnumber, .vfmt)
}
}
if lnumber !in vt.filtered_lines[.trailing_space] {
if line.ends_with(' ') {
vt.error('Looks like you have trailing whitespace.', lnumber, .unknown)
}
}
}

// vet_fn_documentation ensures that functions are documented
Expand Down Expand Up @@ -229,6 +254,62 @@ fn (mut vt Vet) vet_fn_documentation(lines []string, line string, lnumber int) {
}
}

fn (mut vt Vet) stmts(stmts []ast.Stmt) {
for stmt in stmts {
vt.stmt(stmt)
}
}

fn (mut vt Vet) stmt(stmt ast.Stmt) {
match stmt {
ast.ConstDecl { vt.exprs(stmt.fields.map(it.expr)) }
ast.ExprStmt { vt.expr(stmt.expr) }
ast.Return { vt.exprs(stmt.exprs) }
ast.AssertStmt { vt.expr(stmt.expr) }
ast.AssignStmt { vt.exprs(stmt.right) }
ast.FnDecl { vt.stmts(stmt.stmts) }
else {}
}
}

fn (mut vt Vet) exprs(exprs []ast.Expr) {
for expr in exprs {
vt.expr(expr)
}
}

fn (mut vt Vet) expr(expr ast.Expr) {
match expr {
ast.Comment {
vt.filtered_lines.comments(expr.is_multi, expr.pos)
}
ast.StringLiteral, ast.StringInterLiteral {
vt.filtered_lines.assigns(expr.pos)
}
ast.ArrayInit {
vt.filtered_lines.assigns(expr.pos)
}
ast.InfixExpr {
vt.expr(expr.right)
}
ast.CallExpr {
vt.expr(expr.left)
vt.exprs(expr.args.map(it.expr))
}
ast.MatchExpr {
for b in expr.branches {
vt.stmts(b.stmts)
}
}
ast.IfExpr {
for b in expr.branches {
vt.stmts(b.stmts)
}
}
else {}
}
}

fn (vt &Vet) vprintln(s string) {
if !vt.opt.is_verbose {
return
Expand Down
40 changes: 0 additions & 40 deletions vlib/v/parser/parser.v
Original file line number Diff line number Diff line change
Expand Up @@ -278,28 +278,6 @@ pub fn parse_vet_file(path string, mut table_ ast.Table, pref_ &pref.Preferences
warnings: []errors.Warning{}
}
p.set_path(path)
if p.scanner.text.contains_any_substr(['\n ', ' \n']) {
source_lines := os.read_lines(path) or { []string{} }
mut is_vfmt_off := false
for lnumber, line in source_lines {
if line.starts_with('// vfmt off') {
is_vfmt_off = true
} else if line.starts_with('// vfmt on') {
is_vfmt_off = false
}
if is_vfmt_off {
continue
}
if line.starts_with(' ') {
p.vet_error('Looks like you are using spaces for indentation.', lnumber,
.vfmt, .space_indent)
}
if line.ends_with(' ') {
p.vet_error('Looks like you have trailing whitespace.', lnumber, .unknown,
.trailing_space)
}
}
}
p.vet_errors << p.scanner.vet_errors
file := p.parse()
unsafe { p.free_scanner() }
Expand Down Expand Up @@ -910,11 +888,6 @@ fn (mut p Parser) comment() ast.Comment {
is_multi := num_newlines > 0
pos.last_line = pos.line_nr + num_newlines
p.next()
// Filter out false positive space indent vet errors inside comments
if p.vet_errors.len > 0 && is_multi {
p.vet_errors = p.vet_errors.filter(it.typ != .space_indent
|| it.pos.line_nr - 1 > pos.last_line || it.pos.line_nr - 1 <= pos.line_nr)
}
return ast.Comment{
text: text
is_multi: is_multi
Expand Down Expand Up @@ -3467,17 +3440,6 @@ fn (mut p Parser) enum_val() ast.EnumVal {
}
}

fn (mut p Parser) filter_string_vet_errors(pos token.Pos) {
if p.vet_errors.len == 0 {
return
}
p.vet_errors = p.vet_errors.filter(
(it.typ == .trailing_space && it.pos.line_nr - 1 >= pos.last_line)
|| (it.typ != .trailing_space && it.pos.line_nr - 1 > pos.last_line)
|| (it.typ == .space_indent && it.pos.line_nr - 1 <= pos.line_nr)
|| (it.typ != .space_indent && it.pos.line_nr - 1 < pos.line_nr))
}

fn (mut p Parser) string_expr() ast.Expr {
is_raw := p.tok.kind == .name && p.tok.lit == 'r'
is_cstr := p.tok.kind == .name && p.tok.lit == 'c'
Expand All @@ -3490,7 +3452,6 @@ fn (mut p Parser) string_expr() ast.Expr {
pos.last_line = pos.line_nr + val.count('\n')
if p.peek_tok.kind != .str_dollar {
p.next()
p.filter_string_vet_errors(pos)
node = ast.StringLiteral{
val: val
is_raw: is_raw
Expand Down Expand Up @@ -3570,7 +3531,6 @@ fn (mut p Parser) string_expr() ast.Expr {
fposs << p.prev_tok.pos()
}
pos = pos.extend(p.prev_tok.pos())
p.filter_string_vet_errors(pos)
node = ast.StringInterLiteral{
vals: vals
exprs: exprs
Expand Down

0 comments on commit 387af74

Please sign in to comment.