From 290b79c15dbbefcba6309880794fb3d677f7a202 Mon Sep 17 00:00:00 2001 From: Brian Anderson Date: Sat, 24 Jan 2015 15:18:19 -0800 Subject: [PATCH] Clean up tidy scripts, coverage, performance This restructures tidy.py to walk the tree itself, and improves performance considerably by not loading entire files into buffers for licenseck. Splits build rules into 'tidy', 'tidy-basic', 'tidy-binaries', 'tidy-errors', 'tidy-features'. --- mk/tests.mk | 47 +---- src/etc/errorck.py | 4 + src/etc/featureck.py | 4 + src/etc/licenseck.py | 5 - src/etc/tidy.py | 196 ++++++++++++++---- src/grammar/check.sh | 2 + src/test/run-make/c-dynamic-dylib/cfoo.c | 1 + src/test/run-make/c-dynamic-rlib/cfoo.c | 1 + src/test/run-make/c-link-to-rust-dylib/bar.c | 1 + .../run-make/c-link-to-rust-staticlib/bar.c | 1 + src/test/run-make/c-static-dylib/cfoo.c | 1 + src/test/run-make/c-static-rlib/cfoo.c | 1 + src/test/run-make/extern-fn-generic/test.c | 13 +- src/test/run-make/extern-fn-mangle/test.c | 3 +- .../extern-fn-with-packed-struct/test.c | 1 + src/test/run-make/extern-fn-with-union/test.c | 1 + .../run-make/interdependent-c-libraries/bar.c | 1 + .../run-make/interdependent-c-libraries/foo.c | 1 + src/test/run-make/issue-12446/foo.c | 1 + src/test/run-make/issue-15460/foo.c | 1 + src/test/run-make/link-path-order/correct.c | 1 + src/test/run-make/link-path-order/wrong.c | 1 + .../run-make/linkage-attr-on-static/foo.c | 1 + src/test/run-make/lto-smoke-c/bar.c | 1 + src/test/run-make/manual-link/bar.c | 1 + src/test/run-make/manual-link/foo.c | 1 + src/test/run-make/no-duplicate-libs/bar.c | 1 + src/test/run-make/no-duplicate-libs/foo.c | 1 + 28 files changed, 192 insertions(+), 102 deletions(-) diff --git a/mk/tests.mk b/mk/tests.mk index a9abc6120d86..13d12c99dc96 100644 --- a/mk/tests.mk +++ b/mk/tests.mk @@ -235,30 +235,10 @@ cleantestlibs: ###################################################################### ifdef CFG_NOTIDY +.PHONY: tidy tidy: else -ALL_CS := $(wildcard $(S)src/rt/*.cpp \ - $(S)src/rt/*/*.cpp \ - $(S)src/rt/*/*/*.cpp \ - $(S)src/rustllvm/*.cpp) -ALL_CS := $(filter-out $(S)src/rt/miniz.cpp \ - $(wildcard $(S)src/rt/hoedown/src/*.c) \ - $(wildcard $(S)src/rt/hoedown/bin/*.c) \ - ,$(ALL_CS)) -ALL_HS := $(wildcard $(S)src/rt/*.h \ - $(S)src/rt/*/*.h \ - $(S)src/rt/*/*/*.h \ - $(S)src/rustllvm/*.h) -ALL_HS := $(filter-out $(S)src/rt/valgrind/valgrind.h \ - $(S)src/rt/valgrind/memcheck.h \ - $(S)src/rt/msvc/typeof.h \ - $(S)src/rt/msvc/stdint.h \ - $(S)src/rt/msvc/inttypes.h \ - $(wildcard $(S)src/rt/hoedown/src/*.h) \ - $(wildcard $(S)src/rt/hoedown/bin/*.h) \ - ,$(ALL_HS)) - # Run the tidy script in multiple parts to avoid huge 'echo' commands .PHONY: tidy tidy: tidy-basic tidy-binaries tidy-errors tidy-features @@ -268,30 +248,7 @@ endif .PHONY: tidy-basic tidy-basic: @$(call E, check: formatting) - $(Q)find $(S)src -name '*.r[sc]' \ - -and -not -regex '^$(S)src/jemalloc.*' \ - -and -not -regex '^$(S)src/libuv.*' \ - -and -not -regex '^$(S)src/llvm.*' \ - -and -not -regex '^$(S)src/gyp.*' \ - -and -not -regex '^$(S)src/libbacktrace.*' \ - -print0 \ - | xargs -0 -n 10 $(CFG_PYTHON) $(S)src/etc/tidy.py - $(Q)find $(S)src/etc -name '*.py' \ - | xargs -n 10 $(CFG_PYTHON) $(S)src/etc/tidy.py - $(Q)find $(S)src/doc -name '*.js' \ - | xargs -n 10 $(CFG_PYTHON) $(S)src/etc/tidy.py - $(Q)find $(S)src/etc -name '*.sh' \ - | xargs -n 10 $(CFG_PYTHON) $(S)src/etc/tidy.py - $(Q)find $(S)src/etc -name '*.pl' \ - | xargs -n 10 $(CFG_PYTHON) $(S)src/etc/tidy.py - $(Q)find $(S)src/etc -name '*.c' \ - | xargs -n 10 $(CFG_PYTHON) $(S)src/etc/tidy.py - $(Q)find $(S)src/etc -name '*.h' \ - | xargs -n 10 $(CFG_PYTHON) $(S)src/etc/tidy.py - $(Q)echo $(ALL_CS) \ - | xargs -n 10 $(CFG_PYTHON) $(S)src/etc/tidy.py - $(Q)echo $(ALL_HS) \ - | xargs -n 10 $(CFG_PYTHON) $(S)src/etc/tidy.py + $(Q) $(CFG_PYTHON) $(S)src/etc/tidy.py $(S)src/ .PHONY: tidy-binaries tidy-binaries: diff --git a/src/etc/errorck.py b/src/etc/errorck.py index 9db9ed576cbe..7b11504f3cd8 100644 --- a/src/etc/errorck.py +++ b/src/etc/errorck.py @@ -15,6 +15,10 @@ import sys import os import re +if len(sys.argv) < 2: + print "usage: errorck.py " + sys.exit(1) + src_dir = sys.argv[1] errcode_map = {} error_re = re.compile("(E\d\d\d\d)") diff --git a/src/etc/featureck.py b/src/etc/featureck.py index 71a145e4a6eb..df4ea998fefc 100644 --- a/src/etc/featureck.py +++ b/src/etc/featureck.py @@ -20,6 +20,10 @@ import sys, os, re +if len(sys.argv) < 2: + print "usage: featurkck.py " + sys.exit(1) + src_dir = sys.argv[1] # Features that are allowed to exist in both the language and the library diff --git a/src/etc/licenseck.py b/src/etc/licenseck.py index f38583ee1fb3..889b2c95a7ea 100644 --- a/src/etc/licenseck.py +++ b/src/etc/licenseck.py @@ -22,11 +22,6 @@ u"""(#|//) Copyright .* The Rust Project Developers. See the COPYRIGHT \\1 except according to those terms.""") exceptions = [ - "rt/rust_android_dummy.cpp", # BSD, chromium - "rt/rust_android_dummy.h", # BSD, chromium - "rt/isaac/randport.cpp", # public domain - "rt/isaac/rand.h", # public domain - "rt/isaac/standard.h", # public domain "libstd/sync/mpsc/mpsc_queue.rs", # BSD "libstd/sync/mpsc/spsc_queue.rs", # BSD "test/bench/shootout-binarytrees.rs", # BSD diff --git a/src/etc/tidy.py b/src/etc/tidy.py index f5172feb5b60..d055576f430f 100644 --- a/src/etc/tidy.py +++ b/src/etc/tidy.py @@ -51,78 +51,184 @@ def do_license_check(name, contents): if not check_license(name, contents): report_error_name_no(name, 1, "incorrect license") - -file_names = [s for s in sys.argv[1:] if (not s.endswith("_gen.rs")) - and (not ".#" in s)] - current_name = "" current_contents = "" check_tab = True check_cr = True check_linelength = True +if len(sys.argv) < 2: + print "usage: tidy.py " + sys.exit(1) + +src_dir = sys.argv[1] try: - for line in fileinput.input(file_names, + count_rs = 0 + count_py = 0 + count_js = 0 + count_sh = 0 + count_pl = 0 + count_c = 0 + count_h = 0 + count_other = 0 + + count_lines = 0 + count_non_blank_lines = 0 + + def update_counts(current_name): + global count_rs + global count_py + global count_js + global count_sh + global count_pl + global count_c + global count_h + global count_other + + if current_name.endswith(".rs"): + count_rs += 1 + if current_name.endswith(".py"): + count_py += 1 + if current_name.endswith(".js"): + count_js += 1 + if current_name.endswith(".sh"): + count_sh += 1 + if current_name.endswith(".pl"): + count_pl += 1 + if current_name.endswith(".c"): + count_c += 1 + if current_name.endswith(".h"): + count_h += 1 + + all_paths = set() + + for (dirpath, dirnames, filenames) in os.walk(src_dir): + + # Skip some third-party directories + if "src/jemalloc" in dirpath: continue + if "src/llvm" in dirpath: continue + if "src/gyp" in dirpath: continue + if "src/libbacktrace" in dirpath: continue + if "src/compiler-rt" in dirpath: continue + if "src/rt/hoedown" in dirpath: continue + if "src/rustllvm" in dirpath: continue + if "src/rt/valgrind" in dirpath: continue + if "src/rt/msvc" in dirpath: continue + if "src/rust-installer" in dirpath: continue + + def interesting_file(f): + if "miniz.c" in f \ + or "jquery" in f \ + or "rust_android_dummy" in f: + return False + + if f.endswith(".rs") \ + or f.endswith(".py") \ + or f.endswith(".js") \ + or f.endswith(".sh") \ + or f.endswith(".pl") \ + or f.endswith(".c") \ + or f.endswith(".h") : + return True + else: + return False + + file_names = [os.path.join(dirpath, f) for f in filenames + if interesting_file(f) + and not f.endswith("_gen.rs") + and not ".#" is f] + + if not file_names: + continue + + for line in fileinput.input(file_names, openhook=fileinput.hook_encoded("utf-8")): - if "tidy.py" not in fileinput.filename(): + filename = fileinput.filename() + + if "tidy.py" not in filename: + if "TODO" in line: + report_err("TODO is deprecated; use FIXME") + match = re.match(r'^.*/(\*|/!?)\s*XXX', line) + if match: + report_err("XXX is no longer necessary, use FIXME") + match = re.match(r'^.*//\s*(NOTE.*)$', line) + if match and "TRAVIS" not in os.environ: + m = match.group(1) + if "snap" in m.lower(): + report_warn(match.group(1)) + match = re.match(r'^.*//\s*SNAP\s+(\w+)', line) + if match: + hsh = match.group(1) + date, rev = snapshot.curr_snapshot_rev() + if not hsh.startswith(rev): + report_err("snapshot out of date (" + date + + "): " + line) + else: + if "SNAP" in line: + report_warn("unmatched SNAP line: " + line) + if cr_flag in line: check_cr = False if tab_flag in line: check_tab = False if linelength_flag in line: check_linelength = False - if "TODO" in line: - report_err("TODO is deprecated; use FIXME") - match = re.match(r'^.*/(\*|/!?)\s*XXX', line) - if match: - report_err("XXX is no longer necessary, use FIXME") - match = re.match(r'^.*//\s*(NOTE.*)$', line) - if match and "TRAVIS" not in os.environ: - m = match.group(1) - if "snap" in m.lower(): - report_warn(match.group(1)) - match = re.match(r'^.*//\s*SNAP\s+(\w+)', line) - if match: - hsh = match.group(1) - date, rev = snapshot.curr_snapshot_rev() - if not hsh.startswith(rev): - report_err("snapshot out of date (" + date - + "): " + line) - else: - if "SNAP" in line: - report_warn("unmatched SNAP line: " + line) - if check_tab and ('\t' in line and - "Makefile" not in fileinput.filename()): - report_err("tab character") - if check_cr and not autocrlf and '\r' in line: - report_err("CR character") - if line.endswith(" \n") or line.endswith("\t\n"): - report_err("trailing whitespace") - line_len = len(line)-2 if autocrlf else len(line)-1 + if check_tab and ('\t' in line and + "Makefile" not in filename): + report_err("tab character") + if check_cr and not autocrlf and '\r' in line: + report_err("CR character") + if line.endswith(" \n") or line.endswith("\t\n"): + report_err("trailing whitespace") + line_len = len(line)-2 if autocrlf else len(line)-1 - if check_linelength and line_len > cols: - report_err("line longer than %d chars" % cols) + if check_linelength and line_len > cols: + report_err("line longer than %d chars" % cols) - if fileinput.isfirstline() and current_name != "": - do_license_check(current_name, current_contents) + if fileinput.isfirstline(): + # This happens at the end of each file except the last. + if current_name != "": + update_counts(current_name) + assert len(current_contents) > 0 + do_license_check(current_name, current_contents) - if fileinput.isfirstline(): - current_name = fileinput.filename() - current_contents = "" - check_cr = True - check_tab = True - check_linelength = True + current_name = filename + current_contents = "" + check_cr = True + check_tab = True + check_linelength = True - current_contents += line + # Put a reasonable limit on the amount of header data we use for + # the licenseck + if len(current_contents) < 1000: + current_contents += line + + count_lines += 1 + if line.strip(): + count_non_blank_lines += 1 if current_name != "": + update_counts(current_name) + assert len(current_contents) > 0 do_license_check(current_name, current_contents) except UnicodeDecodeError as e: report_err("UTF-8 decoding error " + str(e)) +print +print "* linted .rs files: " + str(count_rs) +print "* linted .py files: " + str(count_py) +print "* linted .js files: " + str(count_js) +print "* linted .sh files: " + str(count_sh) +print "* linted .pl files: " + str(count_pl) +print "* linted .c files: " + str(count_c) +print "* linted .h files: " + str(count_h) +print "* other linted files: " + str(count_other) +print "* total lines of code: " + str(count_lines) +print "* total non-blank lines of code: " + str(count_non_blank_lines) +print sys.exit(err) diff --git a/src/grammar/check.sh b/src/grammar/check.sh index cb269bbdb0ad..b5be3daa13e1 100755 --- a/src/grammar/check.sh +++ b/src/grammar/check.sh @@ -1,5 +1,7 @@ #!/bin/sh +# ignore-license + # Run the reference lexer against libsyntax and compare the tokens and spans. # If "// ignore-lexer-test" is present in the file, it will be ignored. diff --git a/src/test/run-make/c-dynamic-dylib/cfoo.c b/src/test/run-make/c-dynamic-dylib/cfoo.c index 9fe07f82f9ed..113717a776a9 100644 --- a/src/test/run-make/c-dynamic-dylib/cfoo.c +++ b/src/test/run-make/c-dynamic-dylib/cfoo.c @@ -1 +1,2 @@ +// ignore-license int foo() { return 0; } diff --git a/src/test/run-make/c-dynamic-rlib/cfoo.c b/src/test/run-make/c-dynamic-rlib/cfoo.c index 9fe07f82f9ed..113717a776a9 100644 --- a/src/test/run-make/c-dynamic-rlib/cfoo.c +++ b/src/test/run-make/c-dynamic-rlib/cfoo.c @@ -1 +1,2 @@ +// ignore-license int foo() { return 0; } diff --git a/src/test/run-make/c-link-to-rust-dylib/bar.c b/src/test/run-make/c-link-to-rust-dylib/bar.c index bb4036b06e13..5729d411c5bc 100644 --- a/src/test/run-make/c-link-to-rust-dylib/bar.c +++ b/src/test/run-make/c-link-to-rust-dylib/bar.c @@ -1,3 +1,4 @@ +// ignore-license void foo(); int main() { diff --git a/src/test/run-make/c-link-to-rust-staticlib/bar.c b/src/test/run-make/c-link-to-rust-staticlib/bar.c index bb4036b06e13..5729d411c5bc 100644 --- a/src/test/run-make/c-link-to-rust-staticlib/bar.c +++ b/src/test/run-make/c-link-to-rust-staticlib/bar.c @@ -1,3 +1,4 @@ +// ignore-license void foo(); int main() { diff --git a/src/test/run-make/c-static-dylib/cfoo.c b/src/test/run-make/c-static-dylib/cfoo.c index 9fe07f82f9ed..113717a776a9 100644 --- a/src/test/run-make/c-static-dylib/cfoo.c +++ b/src/test/run-make/c-static-dylib/cfoo.c @@ -1 +1,2 @@ +// ignore-license int foo() { return 0; } diff --git a/src/test/run-make/c-static-rlib/cfoo.c b/src/test/run-make/c-static-rlib/cfoo.c index 9fe07f82f9ed..113717a776a9 100644 --- a/src/test/run-make/c-static-rlib/cfoo.c +++ b/src/test/run-make/c-static-rlib/cfoo.c @@ -1 +1,2 @@ +// ignore-license int foo() { return 0; } diff --git a/src/test/run-make/extern-fn-generic/test.c b/src/test/run-make/extern-fn-generic/test.c index f23dd1eb1462..f9faef64afc4 100644 --- a/src/test/run-make/extern-fn-generic/test.c +++ b/src/test/run-make/extern-fn-generic/test.c @@ -1,16 +1,17 @@ +// ignore-license #include typedef struct TestStruct { - uint8_t x; - int32_t y; + uint8_t x; + int32_t y; } TestStruct; typedef int callback(TestStruct s); uint32_t call(callback *c) { - TestStruct s; - s.x = 'a'; - s.y = 3; + TestStruct s; + s.x = 'a'; + s.y = 3; - return c(s); + return c(s); } diff --git a/src/test/run-make/extern-fn-mangle/test.c b/src/test/run-make/extern-fn-mangle/test.c index 8d93917ade03..1a9855dedec4 100644 --- a/src/test/run-make/extern-fn-mangle/test.c +++ b/src/test/run-make/extern-fn-mangle/test.c @@ -1,8 +1,9 @@ +// ignore-license #include uint32_t foo(); uint32_t bar(); uint32_t add() { - return foo() + bar(); + return foo() + bar(); } diff --git a/src/test/run-make/extern-fn-with-packed-struct/test.c b/src/test/run-make/extern-fn-with-packed-struct/test.c index c3456a64b9bc..121e48e84e46 100644 --- a/src/test/run-make/extern-fn-with-packed-struct/test.c +++ b/src/test/run-make/extern-fn-with-packed-struct/test.c @@ -1,3 +1,4 @@ +// ignore-license // Pragma needed cause of gcc bug on windows: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52991 #pragma pack(1) struct __attribute__((packed)) Foo { diff --git a/src/test/run-make/extern-fn-with-union/test.c b/src/test/run-make/extern-fn-with-union/test.c index 86cb64537236..8c87c230693d 100644 --- a/src/test/run-make/extern-fn-with-union/test.c +++ b/src/test/run-make/extern-fn-with-union/test.c @@ -1,3 +1,4 @@ +// ignore-license #include #include diff --git a/src/test/run-make/interdependent-c-libraries/bar.c b/src/test/run-make/interdependent-c-libraries/bar.c index 812c97535287..c761f029effb 100644 --- a/src/test/run-make/interdependent-c-libraries/bar.c +++ b/src/test/run-make/interdependent-c-libraries/bar.c @@ -1,3 +1,4 @@ +// ignore-license void foo(); void bar() { foo(); } diff --git a/src/test/run-make/interdependent-c-libraries/foo.c b/src/test/run-make/interdependent-c-libraries/foo.c index 85e6cd8c3909..2895ad473bf9 100644 --- a/src/test/run-make/interdependent-c-libraries/foo.c +++ b/src/test/run-make/interdependent-c-libraries/foo.c @@ -1 +1,2 @@ +// ignore-license void foo() {} diff --git a/src/test/run-make/issue-12446/foo.c b/src/test/run-make/issue-12446/foo.c index a58cd8bb7c98..186a0046e80a 100644 --- a/src/test/run-make/issue-12446/foo.c +++ b/src/test/run-make/issue-12446/foo.c @@ -1 +1,2 @@ +// ignore-license void some_c_symbol() {} diff --git a/src/test/run-make/issue-15460/foo.c b/src/test/run-make/issue-15460/foo.c index 85e6cd8c3909..2895ad473bf9 100644 --- a/src/test/run-make/issue-15460/foo.c +++ b/src/test/run-make/issue-15460/foo.c @@ -1 +1,2 @@ +// ignore-license void foo() {} diff --git a/src/test/run-make/link-path-order/correct.c b/src/test/run-make/link-path-order/correct.c index 3064af952f89..a595939f92e8 100644 --- a/src/test/run-make/link-path-order/correct.c +++ b/src/test/run-make/link-path-order/correct.c @@ -1 +1,2 @@ +// ignore-license int should_return_one() { return 1; } diff --git a/src/test/run-make/link-path-order/wrong.c b/src/test/run-make/link-path-order/wrong.c index 64275b3ad6bb..c53e7e3c48c0 100644 --- a/src/test/run-make/link-path-order/wrong.c +++ b/src/test/run-make/link-path-order/wrong.c @@ -1 +1,2 @@ +// ignore-license int should_return_one() { return 0; } diff --git a/src/test/run-make/linkage-attr-on-static/foo.c b/src/test/run-make/linkage-attr-on-static/foo.c index 78a6934f57f7..d7d33ea12e80 100644 --- a/src/test/run-make/linkage-attr-on-static/foo.c +++ b/src/test/run-make/linkage-attr-on-static/foo.c @@ -1,3 +1,4 @@ +// ignore-license #include extern int32_t BAZ; diff --git a/src/test/run-make/lto-smoke-c/bar.c b/src/test/run-make/lto-smoke-c/bar.c index bb4036b06e13..5729d411c5bc 100644 --- a/src/test/run-make/lto-smoke-c/bar.c +++ b/src/test/run-make/lto-smoke-c/bar.c @@ -1,3 +1,4 @@ +// ignore-license void foo(); int main() { diff --git a/src/test/run-make/manual-link/bar.c b/src/test/run-make/manual-link/bar.c index e42599986781..3c167b45af98 100644 --- a/src/test/run-make/manual-link/bar.c +++ b/src/test/run-make/manual-link/bar.c @@ -1 +1,2 @@ +// ignore-license void bar() {} diff --git a/src/test/run-make/manual-link/foo.c b/src/test/run-make/manual-link/foo.c index e42599986781..3c167b45af98 100644 --- a/src/test/run-make/manual-link/foo.c +++ b/src/test/run-make/manual-link/foo.c @@ -1 +1,2 @@ +// ignore-license void bar() {} diff --git a/src/test/run-make/no-duplicate-libs/bar.c b/src/test/run-make/no-duplicate-libs/bar.c index 330d914a011e..a7b02a2f10b6 100644 --- a/src/test/run-make/no-duplicate-libs/bar.c +++ b/src/test/run-make/no-duplicate-libs/bar.c @@ -1,3 +1,4 @@ +// ignore-license extern void foo(); void bar() { foo(); } diff --git a/src/test/run-make/no-duplicate-libs/foo.c b/src/test/run-make/no-duplicate-libs/foo.c index 85e6cd8c3909..2895ad473bf9 100644 --- a/src/test/run-make/no-duplicate-libs/foo.c +++ b/src/test/run-make/no-duplicate-libs/foo.c @@ -1 +1,2 @@ +// ignore-license void foo() {}