1 patch for repository http://tahoe-lafs.org/source/tahoe/trunk: Fri Oct 7 08:41:21 BST 2011 david-sarah@jacaranda.org * Add misc/coding_tools/check-miscaptures.py to detect incorrect captures of variables declared in a for loop, and a 'make check-miscaptures' Makefile target to run it. (It is also run by 'make code-checks'.) This is a rewritten version that reports much fewer false positives, by determining captured variables more accurately. fixes #1555 New patches: [Add misc/coding_tools/check-miscaptures.py to detect incorrect captures of variables declared in a for loop, and a 'make check-miscaptures' Makefile target to run it. (It is also run by 'make code-checks'.) This is a rewritten version that reports much fewer false positives, by determining captured variables more accurately. fixes #1555 david-sarah@jacaranda.org**20111007074121 Ignore-this: 51318e9678d132c374ea557ab955e79e ] { hunk ./Makefile 124 false endif -code-checks: build version-and-path check-interfaces -find-trailing-spaces -check-umids pyflakes +code-checks: build version-and-path check-interfaces check-miscaptures -find-trailing-spaces -check-umids pyflakes version-and-path: $(TAHOE) --version-and-path hunk ./Makefile 133 $(TAHOE) @misc/coding_tools/check-interfaces.py 2>&1 |tee violations.txt @echo +check-miscaptures: + $(PYTHON) misc/coding_tools/check-miscaptures.py $(SOURCES) 2>&1 |tee miscaptures.txt + @echo + pyflakes: $(PYTHON) -OOu `which pyflakes` $(SOURCES) |sort |uniq @echo addfile ./misc/coding_tools/check-miscaptures.py hunk ./misc/coding_tools/check-miscaptures.py 1 +#! /usr/bin/python + +import os, sys, compiler, traceback +from compiler.ast import Node, For, AssName, Name, Lambda, Function + + +def check_source(source): + return check_thing(compiler.parse, source) + +def check_file(path): + return check_thing(compiler.parseFile, path) + +def check_thing(parser, thing): + try: + ast = parser(thing) + except SyntaxError, e: + return [e] + else: + results = [] + check_ast(ast, results) + return results + +def check_ast(ast, results): + """Check a node outside a 'for' loop.""" + if isinstance(ast, For): + check_for(ast, results) + else: + for child in ast.getChildNodes(): + if isinstance(ast, Node): + check_ast(child, results) + +def check_for(ast, results): + """Check a particular outer 'for' loop.""" + + declared = {} # maps name to lineno of declaration + nested = set() + collect_declared_and_nested(ast, declared, nested) + + # For each nested function... + for funcnode in nested: + # Check for captured variables in this function. + captured = set() + collect_captured(funcnode, declared, captured) + for name in captured: + # We want to report the outermost capturing function + # (since that is where the workaround will need to be + # added), and the variable declaration. Just one report + # per capturing function per variable will do. + results.append(make_result(funcnode, name, declared[name])) + + # Check each node in the function body in case it + # contains another 'for' loop. + childnodes = funcnode.getChildNodes()[len(funcnode.defaults):] + for child in childnodes: + check_ast(funcnode, results) + +def collect_declared_and_nested(ast, declared, nested): + """ + Collect the names declared in this 'for' loop, not including + names declared in nested functions. Also collect the nodes of + functions that are nested one level deep. + """ + if isinstance(ast, AssName): + declared[ast.name] = ast.lineno + else: + childnodes = ast.getChildNodes() + if isinstance(ast, (Lambda, Function)): + nested.add(ast) + + # The default argument expressions are "outside" the + # function, even though they are children of the + # Lambda or Function node. + childnodes = childnodes[:len(ast.defaults)] + + for child in childnodes: + if isinstance(ast, Node): + collect_declared_and_nested(child, declared, nested) + +def collect_captured(ast, declared, captured): + """Collect any captured variables that are also in declared.""" + if isinstance(ast, Name): + if ast.name in declared: + captured.add(ast.name) + else: + childnodes = ast.getChildNodes() + + if isinstance(ast, (Lambda, Function)): + # Formal parameters of the function are excluded from + # captures we care about in subnodes of the function body. + declared = declared.copy() + for argname in ast.argnames: + if argname in declared: + del declared[argname] + + for child in childnodes[len(ast.defaults):]: + collect_captured(child, declared, captured) + + # The default argument expressions are "outside" the + # function, even though they are children of the + # Lambda or Function node. + childnodes = childnodes[:len(ast.defaults)] + + for child in childnodes: + if isinstance(ast, Node): + collect_captured(child, declared, captured) + + +def make_result(funcnode, var_name, var_lineno): + if hasattr(funcnode, 'name'): + func_name = 'function %r' % (funcnode.name,) + else: + func_name = '' + return (funcnode.lineno, func_name, var_name, var_lineno) + +def report(out, path, results): + for r in results: + if isinstance(r, SyntaxError): + print >>out, path + (" NOT ANALYSED due to syntax error: %s" % r) + else: + print >>out, path + (":%r %s captures %r declared at line %d" % r) + +def check(sources, out): + class Counts: + n = 0 + processed_files = 0 + suspect_files = 0 + counts = Counts() + + def _process(path): + results = check_file(path) + report(out, path, results) + counts.n += len(results) + counts.processed_files += 1 + if len(results) > 0: + counts.suspect_files += 1 + + for source in sources: + print >>out, "Checking %s..." % (source,) + if os.path.isfile(source): + _process(source) + else: + for (dirpath, dirnames, filenames) in os.walk(source): + for fn in filenames: + (basename, ext) = os.path.splitext(fn) + if ext == '.py': + _process(os.path.join(dirpath, fn)) + + print >>out, ("%d suspiciously captured variables in %d out of %d files" + % (counts.n, counts.suspect_files, counts.processed_files)) + return counts.n + + +sources = ['src'] +if len(sys.argv) > 1: + sources = sys.argv[1:] +if check(sources, sys.stderr) > 0: + sys.exit(1) + + +# TODO: self-tests } Context: [no_network.py: Clean up whitespace around code changed by previous patch. david-sarah@jacaranda.org**20111004010407 Ignore-this: 647ec8a9346dca1a41212ab250619b72 ] [no_network.py: Fix potential bugs in some tests due to capture of slots in for loops. david-sarah@jacaranda.org**20111004010231 Ignore-this: 9c496877613a3befd54979e5de6e63d2 ] [docs: fix the rst formatting of COPYING.TGPPL.rst zooko@zooko.com**20111003043333 Ignore-this: c5fbc83f4a3db81a0c95b27053c463c5 Now it renders correctly both on trac and with rst2html --verbose from docutils v0.8.1. ] [MDMF: remove extension fields from caps, tolerate arbitrary ones. Fixes #1526 Brian Warner **20111001233553 Ignore-this: 335e1690aef1146a2c0b8d8c18c1cb21 The filecaps used to be produced with hints for 'k' and segsize, but they weren't actually used, and doing so had the potential to limit how we change those filecaps in the future. Also the parsing code had some problems dealing with other numbers of extensions. Removing the existing fields and making the parser tolerate (and ignore) extra ones makes MDMF more future-proof. ] [test/test_runner.py: BinTahoe.test_path has rare nondeterministic failures; this patch probably fixes a problem where the actual cause of failure is masked by a string conversion error. david-sarah@jacaranda.org**20110927225336 Ignore-this: 6f1ad68004194cc9cea55ace3745e4af ] [docs/configuration.rst: add section about the types of node, and clarify when setting web.port enables web-API service. fixes #1444 zooko@zooko.com**20110926203801 Ignore-this: ab94d470c68e720101a7ff3c207a719e ] [TAG allmydata-tahoe-1.9.0a2 warner@lothar.com**20110925234811 Ignore-this: e9649c58f9c9017a7d55008938dba64f ] Patch bundle hash: 98a38d22095e3489ff7773a45cc873e21893eeb5