Pychecker & pep8 noticed a number of issues (thankfully none are bugs) that are easily fixable, so I went ahead and fixed them.
Diff Detail
Event Timeline
Not sure how you got these changes, but some of them seem wrong and some seem inconsistently applied.
Has this been tested?
C:/LLVM/llvm/tools/clang/utils/analyzer/CmpRuns.py | ||
---|---|---|
194 | PEP 0008: Don't use spaces around the = sign when used to indicate a keyword argument or a default parameter value. |
Whoops, forgot to submit my earlier comments. Responding to comments right now...
C:/LLVM/llvm/tools/clang/utils/analyzer/CmpRuns.py | ||
---|---|---|
31 | This file imports itself? I wasn't sure if I could eliminate this. | |
C:/LLVM/llvm/tools/clang/utils/analyzer/SATestBuild.py | ||
62 | Should this be replaced with multiprocessing.cpu_count? |
Not yet, I was toying with running the first SARD test under it, and read through the code to get an idea of how it works. I remembered that I had PyChecker and pep8 installed, and ran them.
Line length fixes are inconsistently applied. I figured that I'd deal with the worst right now, and deal with the rest later. I can fix all of them in a revised patch if you'd like.
...but are there any functional changes? I didn't intend to make any here.
PyChecker produced the following output for SATestBuild.py:
C:\Users\Alexander Riccio\Downloads>C:\Python27\python.exe C:\Python27\Lib\site-packages\pychecker\checker.py --limit 1000 -Q 1 -P SATestBuild.py [system path]\subprocess.py:406: No doc string for function __init__ [system path]\subprocess.py:410: No doc string for function __str__ SATestBuild.py:67: No module attribute (sysconf_names) found SATestBuild.py:69: No module attribute (sysconf) found SATestBuild.py:73: No global (capture) found SATestBuild.py:114: No doc string for class flushfile SATestBuild.py:114: No doc string for function __init__ SATestBuild.py:116: No doc string for function write SATestBuild.py:122: No doc string for function getProjectMapPath SATestBuild.py:131: No doc string for function getProjectDir SATestBuild.py:134: No doc string for function getSBOutputDirName SATestBuild.py:135: Comparisons with True are not necessary and may not work as expected SATestBuild.py:205: No doc string for function runCleanupScript SATestBuild.py:211: No doc string for function runDownloadScript SATestBuild.py:216: No doc string for function runScript SATestBuild.py:234: No doc string for function downloadAndPatch SATestBuild.py:255: No doc string for function applyPatch SATestBuild.py:275: No doc string for function runScanBuild SATestBuild.py:319: No doc string for function hasNoExtension SATestBuild.py:325: No doc string for function isValidSingleInputFile SATestBuild.py:335: No doc string for function getSDKPath SATestBuild.py:343: No doc string for function runAnalyzePreprocessed SATestBuild.py:374: Comparisons with False are not necessary and may not work as expected SATestBuild.py:397: Comparisons with False are not necessary and may not work as expected SATestBuild.py:400: No doc string for function getBuildLogPath SATestBuild.py:403: No doc string for function removeLogFile SATestBuild.py:412: No doc string for function buildProject SATestBuild.py:469: No doc string for function CleanUpEmptyPlists SATestBuild.py:482: No doc string for function checkBuild SATestBuild.py:506: Local variable (FailuresCopied) not used SATestBuild.py:526: No doc string for class Discarder SATestBuild.py:526: No doc string for function write SATestBuild.py:534: No doc string for function runCmpResults SATestBuild.py:595: No doc string for function cleanupReferenceResults SATestBuild.py:608: No doc string for function updateSVN SATestBuild.py:637: No doc string for function testProject SATestBuild.py:655: Comparisons with False are not necessary and may not work as expected SATestBuild.py:663: No doc string for function testAll SATestBuild.py:678: Comparisons with True are not necessary and may not work as expected SATestBuild.py:679: Comparisons with True are not necessary and may not work as expected SATestBuild.py:688: Comparisons with True are not necessary and may not work as expected
pep8 produced some noisier output for SATestBuild.py:
C:\Users\Alexander Riccio\Downloads>C:\Python27\Scripts\pep8 --ignore=E111 SATestBuild.py SATestBuild.py:6:80: E501 line too long (80 > 79 characters) SATestBuild.py:23:80: E501 line too long (81 > 79 characters) SATestBuild.py:57:1: E265 block comment should start with '# ' SATestBuild.py:59:1: E265 block comment should start with '# ' SATestBuild.py:61:1: E302 expected 2 blank lines, found 1 SATestBuild.py:67:28: W601 .has_key() is deprecated, use 'in' SATestBuild.py:72:14: E261 at least two spaces before inline comment SATestBuild.py:75:18: W601 .has_key() is deprecated, use 'in' SATestBuild.py:79:13: E261 at least two spaces before inline comment SATestBuild.py:81:1: E302 expected 2 blank lines, found 1 SATestBuild.py:81:25: E251 unexpected spaces around keyword / parameter equals SATestBuild.py:81:27: E251 unexpected spaces around keyword / parameter equals SATestBuild.py:86:37: E231 missing whitespace after ',' SATestBuild.py:113:1: E302 expected 2 blank lines, found 1 SATestBuild.py:116:5: E301 expected 1 blank line, found 0 SATestBuild.py:122:1: E302 expected 2 blank lines, found 1 SATestBuild.py:127:17: E126 continuation line over-indented for hanging indent SATestBuild.py:131:1: E302 expected 2 blank lines, found 1 SATestBuild.py:134:1: E302 expected 2 blank lines, found 1 SATestBuild.py:134:41: E203 whitespace before ':' SATestBuild.py:135:25: E712 comparison to True should be 'if cond is True:' or 'if cond:' SATestBuild.py:135:32: E203 whitespace before ':' SATestBuild.py:137:9: E203 whitespace before ':' SATestBuild.py:140:1: E265 block comment should start with '# ' SATestBuild.py:142:1: E265 block comment should start with '# ' SATestBuild.py:178:80: E501 line too long (80 > 79 characters) SATestBuild.py:196:9: E225 missing whitespace around operator SATestBuild.py:196:80: E501 line too long (122 > 79 characters) SATestBuild.py:200:1: E265 block comment should start with '# ' SATestBuild.py:202:1: E265 block comment should start with '# ' SATestBuild.py:205:1: E302 expected 2 blank lines, found 1 SATestBuild.py:211:1: E302 expected 2 blank lines, found 1 SATestBuild.py:216:1: E302 expected 2 blank lines, found 1 SATestBuild.py:221:57: E251 unexpected spaces around keyword / parameter equals SATestBuild.py:221:59: E251 unexpected spaces around keyword / parameter equals SATestBuild.py:222:47: E127 continuation line over-indented for visual indent SATestBuild.py:223:47: E127 continuation line over-indented for visual indent SATestBuild.py:224:47: E127 continuation line over-indented for visual indent SATestBuild.py:225:48: E251 unexpected spaces around keyword / parameter equals SATestBuild.py:225:50: E251 unexpected spaces around keyword / parameter equals SATestBuild.py:226:47: E127 continuation line over-indented for visual indent SATestBuild.py:227:47: E127 continuation line over-indented for visual indent SATestBuild.py:229:80: E501 line too long (80 > 79 characters) SATestBuild.py:230:17: E128 continuation line under-indented for visual indent SATestBuild.py:234:1: E302 expected 2 blank lines, found 1 SATestBuild.py:255:1: E302 expected 2 blank lines, found 1 SATestBuild.py:265:21: E127 continuation line over-indented for visual indent SATestBuild.py:265:24: E251 unexpected spaces around keyword / parameter equals SATestBuild.py:265:26: E251 unexpected spaces around keyword / parameter equals SATestBuild.py:266:21: E127 continuation line over-indented for visual indent SATestBuild.py:267:21: E127 continuation line over-indented for visual indent SATestBuild.py:268:21: E127 continuation line over-indented for visual indent SATestBuild.py:275:1: E302 expected 2 blank lines, found 1 SATestBuild.py:282:18: W601 .has_key() is deprecated, use 'in' SATestBuild.py:288:41: E222 multiple spaces after operator SATestBuild.py:301:25: E703 statement ends with a semicolon SATestBuild.py:306:17: E125 continuation line with same indent as next logical line SATestBuild.py:311:38: E251 unexpected spaces around keyword / parameter equals SATestBuild.py:311:40: E251 unexpected spaces around keyword / parameter equals SATestBuild.py:312:48: E127 continuation line over-indented for visual indent SATestBuild.py:313:48: E127 continuation line over-indented for visual indent SATestBuild.py:315:47: E231 missing whitespace after ',' SATestBuild.py:319:1: E302 expected 2 blank lines, found 1 SATestBuild.py:321:21: E203 whitespace before ':' SATestBuild.py:325:1: E302 expected 2 blank lines, found 1 SATestBuild.py:329:9: E129 visually indented line with same indent as next logical line SATestBuild.py:329:37: E203 whitespace before ':' SATestBuild.py:335:1: E302 expected 2 blank lines, found 1 SATestBuild.py:343:1: E302 expected 2 blank lines, found 1 SATestBuild.py:346:16: E126 continuation line over-indented for hanging indent SATestBuild.py:358:51: E225 missing whitespace around operator SATestBuild.py:360:19: E203 whitespace before ':' SATestBuild.py:364:51: E703 statement ends with a semicolon SATestBuild.py:365:26: E703 statement ends with a semicolon SATestBuild.py:374:46: E712 comparison to False should be 'if cond is False:' or 'if not cond:' SATestBuild.py:385:36: E251 unexpected spaces around keyword / parameter equals SATestBuild.py:385:38: E251 unexpected spaces around keyword / parameter equals SATestBuild.py:386:44: E127 continuation line over-indented for visual indent SATestBuild.py:387:44: E127 continuation line over-indented for visual indent SATestBuild.py:391:20: E126 continuation line over-indented for hanging indent SATestBuild.py:397:19: E712 comparison to False should be 'if cond is False:' or 'if not cond:' SATestBuild.py:398:36: E703 statement ends with a semicolon SATestBuild.py:400:1: E302 expected 2 blank lines, found 1 SATestBuild.py:403:1: E302 expected 2 blank lines, found 1 SATestBuild.py:406:36: E203 whitespace before ':' SATestBuild.py:412:1: E302 expected 2 blank lines, found 1 SATestBuild.py:417:35: E225 missing whitespace around operator SATestBuild.py:422:37: E203 whitespace before ':' SATestBuild.py:442:28: E203 whitespace before ':' SATestBuild.py:455:56: E226 missing whitespace around arithmetic operator SATestBuild.py:455:60: E502 the backslash is redundant between brackets SATestBuild.py:456:31: E127 continuation line over-indented for visual indent SATestBuild.py:456:67: E502 the backslash is redundant between brackets SATestBuild.py:457:31: E127 continuation line over-indented for visual indent SATestBuild.py:465:12: E127 continuation line over-indented for visual indent SATestBuild.py:465:25: E226 missing whitespace around arithmetic operator SATestBuild.py:469:1: E302 expected 2 blank lines, found 1 SATestBuild.py:482:1: E302 expected 2 blank lines, found 1 SATestBuild.py:485:32: E703 statement ends with a semicolon SATestBuild.py:490:12: E127 continuation line over-indented for visual indent SATestBuild.py:491:15: E703 statement ends with a semicolon SATestBuild.py:494:80: E501 line too long (83 > 79 characters) SATestBuild.py:503:52: E127 continuation line over-indented for visual indent SATestBuild.py:510:22: E703 statement ends with a semicolon SATestBuild.py:512:70: E703 statement ends with a semicolon SATestBuild.py:513:47: E703 statement ends with a semicolon SATestBuild.py:515:57: E703 statement ends with a semicolon SATestBuild.py:525:1: E302 expected 2 blank lines, found 1 SATestBuild.py:527:13: E261 at least two spaces before inline comment SATestBuild.py:534:1: E302 expected 2 blank lines, found 1 SATestBuild.py:534:34: E251 unexpected spaces around keyword / parameter equals SATestBuild.py:534:36: E251 unexpected spaces around keyword / parameter equals SATestBuild.py:575:80: E501 line too long (82 > 79 characters) SATestBuild.py:582:26: E203 whitespace before ':' SATestBuild.py:589:80: E501 line too long (82 > 79 characters) SATestBuild.py:592:72: E226 missing whitespace around arithmetic operator SATestBuild.py:595:1: E302 expected 2 blank lines, found 1 SATestBuild.py:608:1: E302 expected 2 blank lines, found 1 SATestBuild.py:637:1: E302 expected 2 blank lines, found 1 SATestBuild.py:637:80: E501 line too long (88 > 79 characters) SATestBuild.py:637:83: E251 unexpected spaces around keyword / parameter equals SATestBuild.py:637:85: E251 unexpected spaces around keyword / parameter equals SATestBuild.py:642:19: E203 whitespace before ':' SATestBuild.py:655:25: E712 comparison to False should be 'if cond is False:' or 'if not cond:' SATestBuild.py:661:28: E226 missing whitespace around arithmetic operator SATestBuild.py:663:1: E302 expected 2 blank lines, found 1 SATestBuild.py:663:29: E251 unexpected spaces around keyword / parameter equals SATestBuild.py:663:31: E251 unexpected spaces around keyword / parameter equals SATestBuild.py:663:48: E251 unexpected spaces around keyword / parameter equals SATestBuild.py:663:50: E251 unexpected spaces around keyword / parameter equals SATestBuild.py:663:68: E251 unexpected spaces around keyword / parameter equals SATestBuild.py:663:70: E251 unexpected spaces around keyword / parameter equals SATestBuild.py:668:29: E203 whitespace before ':' SATestBuild.py:669:80: E501 line too long (80 > 79 characters) SATestBuild.py:678:22: E712 comparison to True should be 'if cond is True:' or 'if cond:' SATestBuild.py:679:37: E712 comparison to True should be 'if cond is True:' or 'if cond:' SATestBuild.py:679:45: E703 statement ends with a semicolon SATestBuild.py:680:32: E241 multiple spaces after ',' SATestBuild.py:680:43: E703 statement ends with a semicolon SATestBuild.py:688:22: E712 comparison to True should be 'if cond is True:' or 'if cond:' SATestBuild.py:689:29: E241 multiple spaces after ',' SATestBuild.py:689:40: E703 statement ends with a semicolon SATestBuild.py:699:80: E501 line too long (83 > 79 characters) SATestBuild.py:701:24: E128 continuation line under-indented for visual indent SATestBuild.py:702:80: E501 line too long (83 > 79 characters) SATestBuild.py:703:80: E501 line too long (87 > 79 characters) SATestBuild.py:704:80: E501 line too long (84 > 79 characters) SATestBuild.py:707:80: E501 line too long (90 > 79 characters)
I ignored the "No doc string for function[...]" and "E265 block comment should start with '# '" warnings entirely.
C:/LLVM/llvm/tools/clang/utils/analyzer/CmpRuns.py | ||
---|---|---|
194 | ...agreed? Huh? |
I've added a few comments where I think the changes are not quite clear.
C:/LLVM/llvm/tools/clang/utils/analyzer/CmpRuns.py | ||
---|---|---|
159 | This was to fix E711 comparison to None should be 'if cond is None:'. | |
C:/LLVM/llvm/tools/clang/utils/analyzer/SATestBuild.py | ||
68 | This was to fix W601 .has_key() is deprecated, use 'in'. | |
317 | This funny looking change was to fix E125 continuation line with same indent as next logical line. | |
341 | I replaced | with or here because I'm fairly certain we want the logical or, not the bitwise or. | |
410 | This was to fix SATestBuild.py:397:19: E712 comparison to False should be 'if cond is False:' or 'if not cond:' | |
471 | This was to fix E502 the backslash is redundant between brackets. | |
679 | This was also to fix E712 comparison to False should be 'if cond is False:' or 'if not cond:'. | |
703 | This was to fix E712 comparison to True should be 'if cond is True:' or 'if cond:'. |
C:/LLVM/llvm/tools/clang/utils/analyzer/SATestBuild.py | ||
---|---|---|
521 | Nope. Good catch! |
This file imports itself?
I wasn't sure if I could eliminate this.