From b3094b0667ecbd79de600bcffddf22c5f91e370e Mon Sep 17 00:00:00 2001 From: Leo Developer Date: Sun, 15 Aug 2021 12:41:51 +0200 Subject: [PATCH] checker: abort prematurely on too many errors (#11185) --- cmd/tools/vcomplete.v | 1 - cmd/v/help/build.txt | 7 ++++--- vlib/v/builder/builder.v | 34 ++++++++++---------------------- vlib/v/checker/checker.v | 42 ++++++++++++++++++++++++++-------------- vlib/v/parser/parser.v | 29 ++++++++++++++++++--------- vlib/v/pref/pref.v | 10 +++------- vlib/v/scanner/scanner.v | 39 +++++++++++++++++++++---------------- 7 files changed, 87 insertions(+), 75 deletions(-) diff --git a/cmd/tools/vcomplete.v b/cmd/tools/vcomplete.v index 5754ce1ea6..8e61755a75 100644 --- a/cmd/tools/vcomplete.v +++ b/cmd/tools/vcomplete.v @@ -131,7 +131,6 @@ const ( '-w', '-print-v-files', '-error-limit', - '-warn-error-limit', '-os', '-printfn', '-cflags', diff --git a/cmd/v/help/build.txt b/cmd/v/help/build.txt index 2916bca33b..fa2a0b2d40 100644 --- a/cmd/v/help/build.txt +++ b/cmd/v/help/build.txt @@ -82,9 +82,10 @@ NB: the build flags are shared with the run command too: d) the function name NB: if you want to output the profile info to stdout, use `-profile -`. - -warn-error-limit - Limit of warnings / errors that will be accumulated (defaults to 100). - Settings this to a negative value will disable the limit. + -error-limit + The maximum amount of warnings / errors / notices, that will be accumulated (defaults to 100). + The checker will abort prematurely once this limit has been reached. + Setting this to 0 or a negative value, will disable the limit. -profile-no-inline Skip [inline] functions when profiling. diff --git a/vlib/v/builder/builder.v b/vlib/v/builder/builder.v index dff86d4010..12998b24c1 100644 --- a/vlib/v/builder/builder.v +++ b/vlib/v/builder/builder.v @@ -19,14 +19,13 @@ pub: compiled_dir string // contains os.real_path() of the dir of the final file beeing compiled, or the dir itself when doing `v .` module_path string mut: - pref &pref.Preferences - checker &checker.Checker - transformer &transformer.Transformer - out_name_c string - out_name_js string - max_nr_errors int = 100 - stats_lines int // size of backend generated source code in lines - stats_bytes int // size of backend generated source code in bytes + pref &pref.Preferences + checker &checker.Checker + transformer &transformer.Transformer + out_name_c string + out_name_js string + stats_lines int // size of backend generated source code in lines + stats_bytes int // size of backend generated source code in bytes pub mut: module_search_paths []string parsed_files []&ast.File @@ -64,10 +63,8 @@ pub fn new_builder(pref &pref.Preferences) Builder { checker: checker.new_checker(table, pref) transformer: transformer.new_transformer(pref) compiled_dir: compiled_dir - max_nr_errors: if pref.error_limit > 0 { pref.error_limit } else { 100 } cached_msvc: msvc } - // max_nr_errors: pref.error_limit ?? 100 TODO potential syntax? } pub fn (mut b Builder) front_stages(v_files []string) ? { @@ -366,7 +363,7 @@ fn (b &Builder) print_warnings_and_errors() { println('$b.checker.nr_notices notices') } if b.checker.nr_notices > 0 && !b.pref.skip_warnings { - for i, err in b.checker.notices { + for err in b.checker.notices { kind := if b.pref.is_verbose { '$err.reporter notice #$b.checker.nr_notices:' } else { @@ -377,13 +374,10 @@ fn (b &Builder) print_warnings_and_errors() { if err.details.len > 0 { eprintln('Details: $err.details') } - if i > b.max_nr_errors { - return - } } } if b.checker.nr_warnings > 0 && !b.pref.skip_warnings { - for i, err in b.checker.warnings { + for err in b.checker.warnings { kind := if b.pref.is_verbose { '$err.reporter warning #$b.checker.nr_warnings:' } else { @@ -394,10 +388,6 @@ fn (b &Builder) print_warnings_and_errors() { if err.details.len > 0 { eprintln('Details: $err.details') } - // eprintln('') - if i > b.max_nr_errors { - return - } } } // @@ -405,7 +395,7 @@ fn (b &Builder) print_warnings_and_errors() { println('$b.checker.nr_errors errors') } if b.checker.nr_errors > 0 { - for i, err in b.checker.errors { + for err in b.checker.errors { kind := if b.pref.is_verbose { '$err.reporter error #$b.checker.nr_errors:' } else { @@ -416,10 +406,6 @@ fn (b &Builder) print_warnings_and_errors() { if err.details.len > 0 { eprintln('Details: $err.details') } - // eprintln('') - if i > b.max_nr_errors { - return - } } b.show_total_warns_and_errors_stats() exit(1) diff --git a/vlib/v/checker/checker.v b/vlib/v/checker/checker.v index 33beed6ed3..53005527d7 100644 --- a/vlib/v/checker/checker.v +++ b/vlib/v/checker/checker.v @@ -53,6 +53,7 @@ pub mut: nr_errors int nr_warnings int nr_notices int + should_abort bool // when too many errors/warnings/notices are accumulated, .should_abort becomes true. It is checked in statement/expression loops, so the checker can return early, instead of wasting time. errors []errors.Error warnings []errors.Warning notices []errors.Notice @@ -139,18 +140,27 @@ pub fn (mut c Checker) check(ast_file &ast.File) { c.expr_level = 0 c.stmt(stmt) } + if c.should_abort { + return + } } for mut stmt in ast_file.stmts { if stmt is ast.GlobalDecl { c.expr_level = 0 c.stmt(stmt) } + if c.should_abort { + return + } } for mut stmt in ast_file.stmts { if stmt !is ast.ConstDecl && stmt !is ast.GlobalDecl && stmt !is ast.ExprStmt { c.expr_level = 0 c.stmt(stmt) } + if c.should_abort { + return + } } c.check_scope_vars(c.file.scope) } @@ -7593,7 +7603,8 @@ fn (c &Checker) check_struct_signature(from ast.Struct, to ast.Struct) bool { } pub fn (mut c Checker) note(message string, pos token.Position) { - if c.nr_notices >= c.pref.warn_error_limit && c.pref.warn_error_limit >= 0 { + if c.pref.message_limit >= 0 && c.nr_notices >= c.pref.message_limit { + c.should_abort = true return } mut details := '' @@ -7625,7 +7636,8 @@ fn (mut c Checker) warn_or_error(message string, pos token.Position, warn bool) } if warn && !c.pref.skip_warnings { c.nr_warnings++ - if c.nr_warnings >= c.pref.warn_error_limit && c.pref.warn_error_limit >= 0 { + if c.pref.message_limit >= 0 && c.nr_warnings >= c.pref.message_limit { + c.should_abort = true return } wrn := errors.Warning{ @@ -7644,19 +7656,21 @@ fn (mut c Checker) warn_or_error(message string, pos token.Position, warn bool) exit(1) } c.nr_errors++ - if c.errors.len < c.pref.warn_error_limit || c.pref.warn_error_limit < 0 { - if pos.line_nr !in c.error_lines { - err := errors.Error{ - reporter: errors.Reporter.checker - pos: pos - file_path: c.file.path - message: message - details: details - } - c.file.errors << err - c.errors << err - c.error_lines << pos.line_nr + if c.pref.message_limit >= 0 && c.errors.len >= c.pref.message_limit { + c.should_abort = true + return + } + if pos.line_nr !in c.error_lines { + err := errors.Error{ + reporter: errors.Reporter.checker + pos: pos + file_path: c.file.path + message: message + details: details } + c.file.errors << err + c.errors << err + c.error_lines << pos.line_nr } } } diff --git a/vlib/v/parser/parser.v b/vlib/v/parser/parser.v index e6248555b1..0924abe5c5 100644 --- a/vlib/v/parser/parser.v +++ b/vlib/v/parser/parser.v @@ -79,6 +79,7 @@ mut: inside_defer bool comp_if_cond bool defer_vars []ast.Ident + should_abort bool // when too many errors/warnings/notices are accumulated, should_abort becomes true, and the parser should stop } // for tests @@ -267,6 +268,9 @@ pub fn (mut p Parser) parse() &ast.File { p.attrs = [] } stmts << stmt + if p.should_abort { + break + } } p.scope.end_pos = p.tok.pos return &ast.File{ @@ -579,6 +583,9 @@ pub fn (mut p Parser) top_stmt() ast.Stmt { } } } + if p.should_abort { + break + } } // TODO remove dummy return statement // the compiler complains if it's not there @@ -1649,9 +1656,11 @@ pub fn (mut p Parser) error_with_error(error errors.Error) { eprintln(ferror) exit(1) } else { - if p.errors.len < p.pref.warn_error_limit || p.pref.warn_error_limit < 0 { - p.errors << error + if p.pref.message_limit >= 0 && p.errors.len >= p.pref.message_limit { + p.should_abort = true + return } + p.errors << error } if p.pref.output_mode == .silent { // Normally, parser errors mean that the parser exits immediately, so there can be only 1 parser error. @@ -1674,13 +1683,15 @@ pub fn (mut p Parser) warn_with_pos(s string, pos token.Position) { ferror := util.formatted_error('warning:', s, p.file_name, pos) eprintln(ferror) } else { - if p.warnings.len < p.pref.warn_error_limit || p.pref.warn_error_limit < 0 { - p.warnings << errors.Warning{ - file_path: p.file_name - pos: pos - reporter: .parser - message: s - } + if p.pref.message_limit >= 0 && p.warnings.len >= p.pref.message_limit { + p.should_abort = true + return + } + p.warnings << errors.Warning{ + file_path: p.file_name + pos: pos + reporter: .parser + message: s } } } diff --git a/vlib/v/pref/pref.v b/vlib/v/pref/pref.v index 234ca70d10..d73d4a47ef 100644 --- a/vlib/v/pref/pref.v +++ b/vlib/v/pref/pref.v @@ -176,7 +176,6 @@ pub mut: no_std bool // when true, do not pass -std=c99 to the C backend use_color ColorOutput // whether the warnings/errors should use ANSI color escapes. is_parallel bool - error_limit int is_vweb bool // skip _ var warning in templates only_check_syntax bool // when true, just parse the files, then stop, before running checker experimental bool // enable experimental features @@ -191,7 +190,7 @@ pub mut: gc_mode GarbageCollectionMode = .no_gc // .no_gc, .boehm, .boehm_leak, ... is_cstrict bool // turn on more C warnings; slightly slower assert_failure_mode AssertFailureMode // whether to call abort() or print_backtrace() after an assertion failure - warn_error_limit int = 100 // limit of warnings/errors to be accumulated + message_limit int = 100 // the maximum amount of warnings/errors/notices that will be accumulated // checker settings: checker_match_exhaustive_cutoff_limit int = 12 } @@ -495,9 +494,6 @@ pub fn parse_args(known_external_commands []string, args []string) (&Preferences '-print-v-files' { res.print_v_files = true } - '-error-limit' { - res.error_limit = cmdline.option(current_args, '-error-limit', '0').int() - } '-os' { target_os := cmdline.option(current_args, '-os', '') i++ @@ -528,8 +524,8 @@ pub fn parse_args(known_external_commands []string, args []string) (&Preferences } i++ } - '-warn-error-limit' { - res.warn_error_limit = cmdline.option(current_args, arg, '10').int() + '-message-limit' { + res.message_limit = cmdline.option(current_args, arg, '5').int() i++ } '-cc' { diff --git a/vlib/v/scanner/scanner.v b/vlib/v/scanner/scanner.v index 3bcccedc1a..87a2a385b6 100644 --- a/vlib/v/scanner/scanner.v +++ b/vlib/v/scanner/scanner.v @@ -57,6 +57,7 @@ pub mut: warnings []errors.Warning notices []errors.Notice vet_errors []vet.Error + should_abort bool // when too many errors/warnings/notices are accumulated, should_abort becomes true, and the scanner should stop } /* @@ -564,7 +565,7 @@ pub fn (mut s Scanner) scan_remaining_text() { continue } s.all_tokens << t - if t.kind == .eof { + if t.kind == .eof || s.should_abort { break } } @@ -579,7 +580,7 @@ pub fn (mut s Scanner) buffer_scan() token.Token { for { cidx := s.tidx s.tidx++ - if cidx >= s.all_tokens.len { + if cidx >= s.all_tokens.len || s.should_abort { return s.end_of_file() } if s.all_tokens[cidx].kind == .comment { @@ -635,7 +636,7 @@ fn (mut s Scanner) text_scan() token.Token { if !s.is_inside_string { s.skip_whitespace() } - if s.pos >= s.text.len { + if s.pos >= s.text.len || s.should_abort { return s.end_of_file() } // End of $var, start next string @@ -1354,13 +1355,15 @@ pub fn (mut s Scanner) warn(msg string) { if s.pref.output_mode == .stdout { eprintln(util.formatted_error('warning:', msg, s.file_path, pos)) } else { - if s.warnings.len < s.pref.warn_error_limit || s.pref.warn_error_limit < 0 { - s.warnings << errors.Warning{ - file_path: s.file_path - pos: pos - reporter: .scanner - message: msg - } + if s.pref.message_limit >= 0 && s.warnings.len >= s.pref.message_limit { + s.should_abort = true + return + } + s.warnings << errors.Warning{ + file_path: s.file_path + pos: pos + reporter: .scanner + message: msg } } } @@ -1378,13 +1381,15 @@ pub fn (mut s Scanner) error(msg string) { if s.pref.fatal_errors { exit(1) } - if s.errors.len < s.pref.warn_error_limit || s.pref.warn_error_limit < 0 { - s.errors << errors.Error{ - file_path: s.file_path - pos: pos - reporter: .scanner - message: msg - } + if s.pref.message_limit >= 0 && s.errors.len >= s.pref.message_limit { + s.should_abort = true + return + } + s.errors << errors.Error{ + file_path: s.file_path + pos: pos + reporter: .scanner + message: msg } } }