[PATCH] D17253: Cleanup of analyzer scripts as suggested by pychecker and pep8

Alexander Riccio via cfe-commits cfe-commits at lists.llvm.org
Mon Feb 15 14:30:12 PST 2016


ariccio added a comment.

I've added a few comments where I think the changes are not quite clear.


================
Comment at: C:/LLVM/llvm/tools/clang/utils/analyzer/CmpRuns.py:159
@@ -147,3 +158,3 @@
         if 'clang_version' in data:
-            if self.clang_version == None:
+            if self.clang_version is None:
                 self.clang_version = data.pop('clang_version')
----------------
This was to fix `E711 comparison to None should be 'if cond is None:'`.

================
Comment at: C:/LLVM/llvm/tools/clang/utils/analyzer/SATestBuild.py:68
@@ -66,3 +67,3 @@
     if hasattr(os, "sysconf"):
-        if os.sysconf_names.has_key("SC_NPROCESSORS_ONLN"):
+        if "SC_NPROCESSORS_ONLN" in os.sysconf_names:
             # Linux & Unix:
----------------
This was to fix `W601 .has_key() is deprecated, use 'in'`.

================
Comment at: C:/LLVM/llvm/tools/clang/utils/analyzer/SATestBuild.py:317
@@ -305,3 +316,3 @@
             if (Command.startswith("make ") or Command == "make") and \
-                "-j" not in Command:
+                    "-j" not in Command:
                 Command += " -j%d" % Jobs
----------------
This funny looking change was to fix `E125 continuation line with same indent as next logical line`.

================
Comment at: C:/LLVM/llvm/tools/clang/utils/analyzer/SATestBuild.py:341
@@ -330,1 +340,3 @@
+            (Ext == ".c") or (Ext == ".cpp") or
+            (Ext == ".m") or (Ext == "")):
         return True
----------------
I replaced `|` with `or` here because I'm fairly certain we want the //logical// `or`, not the //bitwise// `or`.

================
Comment at: C:/LLVM/llvm/tools/clang/utils/analyzer/SATestBuild.py:410
@@ -396,3 +409,3 @@
         # If command did not fail, erase the log file.
-        if Failed == False:
-            os.remove(LogFile.name);
+        if not Failed:
+            os.remove(LogFile.name)
----------------
This was to fix `SATestBuild.py:397:19: E712 comparison to False should be 'if cond is False:' or 'if not cond:'`

================
Comment at: C:/LLVM/llvm/tools/clang/utils/analyzer/SATestBuild.py:471
@@ -454,4 +470,3 @@
                         PathPrefix = os.path.join(Dir, PatchedSourceDirName)
-                    Paths = [SourceFile[len(PathPrefix)+1:]\
-                              if SourceFile.startswith(PathPrefix)\
-                              else SourceFile for SourceFile in Data['files']]
+                    Paths = [SourceFile[len(PathPrefix) + 1:]
+                             if SourceFile.startswith(PathPrefix)
----------------
This was to fix `E502 the backslash is redundant between brackets`.

================
Comment at: C:/LLVM/llvm/tools/clang/utils/analyzer/SATestBuild.py:679
@@ -654,3 +678,3 @@
 
-    if IsReferenceBuild == False:
+    if not IsReferenceBuild:
         runCmpResults(Dir, Strictness)
----------------
This was also to fix `E712 comparison to False should be 'if cond is False:' or 'if not cond:'`.

================
Comment at: C:/LLVM/llvm/tools/clang/utils/analyzer/SATestBuild.py:703
@@ -677,4 +702,3 @@
         # update svn. Remove reference results from SVN.
-        if UpdateSVN == True:
-            assert(IsReferenceBuild == True);
-            updateSVN("delete",  PMapFile);
+        if UpdateSVN:
+            assert(IsReferenceBuild)
----------------
This was to fix `E712 comparison to True should be 'if cond is True:' or 'if cond:'`.


http://reviews.llvm.org/D17253





More information about the cfe-commits mailing list