Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

vet, parser: rewrite vet error handling (improve parser performance extend vvet) p1 #21417

Merged
merged 2 commits into from
May 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading