Move some cppcheck rules to cppcheck addon instead

CMakeLists.txt

@@ -261,10 +261,13 @@ rocm_enable_cppcheck(
         ctuOneDefinitionRuleViolation:*test/*
         useSmartPointer:*src/api/api.cpp
         useSmartPointer:*make_shared_array.hpp
+        migraphx-RedundantLocalVariable:*src/api/api.cpp
     FORCE
     INCONCLUSIVE
     RULE_FILE
-        ${CMAKE_CURRENT_SOURCE_DIR}/cppcheck.rules
+        ${CMAKE_CURRENT_SOURCE_DIR}/tools/cppcheck/rules.xml
+    ADDONS
+        ${CMAKE_CURRENT_SOURCE_DIR}/tools/cppcheck/migraphx.py
     SOURCES
         examples/
         src/

tools/cppcheck/migraphx.py

+import cppcheck, itertools
+from cppcheckdata import simpleMatch, match
+
+def skipTokenMatches(tokens, skip=None):
+    for tok in tokens:
+        if match(tok, skip):
+            continue
+        yield tok
+
+def isTokensEqual(xtokens, ytokens, skip=None):
+    for x, y in itertools.zip_longest(skipTokenMatches(xtokens, skip), skipTokenMatches(ytokens, skip)):
+        if not x:
+            return False
+        if not y:
+            return False
+        if x.str != y.str:
+            return False
+    return True
+
+def getInnerLink(token):
+    if not token:
+        return []
+    if not token.link:
+        return []
+    return token.next.forward(token.link)
+
+def getVariableDecl(var):
+    if not var:
+        return []
+    end = var.typeEndToken
+    if end:
+        end = end.next
+    return var.typeStartToken.forward(end)
+
+@cppcheck.checker
+def AvoidBranchingStatementAsLastInLoop(cfg, data):
+    for token in cfg.tokenlist:
+        end = match(token, "for|while (*) {*}").end
+        if not end:
+            continue
+        stmt = end.tokAt(-2)
+        if not match(stmt, "%any% ; }"):
+            continue
+        if not match(stmt, "break|continue"):
+            stmt = stmt.astTop()
+        if match(stmt, "break|continue|return"):
+            cppcheck.reportError(stmt, "style", "Branching statement as the last statement inside a loop is very confusing.")
+
+# @cppcheck.checker
+# def CollapsibleIfStatements(cfg, data):
+#     for token in cfg.tokenlist:
+#         if not match(token, "if (*) { if (*) {*} }"):
+#             continue
+#         cppcheck.reportError(token, "style", "These two if statements can be collapsed into one.")
+
+@cppcheck.checker
+def ConditionalAssert(cfg, data):
+    for token in cfg.tokenlist:
+        if not match(token, "if (*) { assert (*) ; }"):
+            continue
+        cppcheck.reportError(token, "style", "The if condition should be included in assert.")
+
+@cppcheck.checker
+def EmptyCatchStatement(cfg, data):
+    for token in cfg.tokenlist:
+        if not match(token, "catch (*) { }"):
+            continue
+        cppcheck.reportError(token, "style", "An empty catch statement.")
+
+@cppcheck.checker
+def EmptyDoWhileStatement(cfg, data):
+    for token in cfg.tokenlist:
+        if not match(token, "do { } while ("):
+            continue
+        cppcheck.reportError(token, "style", "Empty do-while.")
+
+@cppcheck.checker
+def EmptyElseBlock(cfg, data):
+    for token in cfg.tokenlist:
+        if not match(token, "else { }"):
+            continue
+        cppcheck.reportError(token, "style", "Empty else statement can be safely removed.")
+
+@cppcheck.checker
+def EmptyForStatement(cfg, data):
+    for token in cfg.tokenlist:
+        if not match(token, "for (*) { }"):
+            continue
+        cppcheck.reportError(token, "style", "Empty for statement.")
+
+@cppcheck.checker
+def EmptyIfStatement(cfg, data):
+    for token in cfg.tokenlist:
+        if not match(token, "if (*) { }"):
+            continue
+        cppcheck.reportError(token, "style", "Empty if statement.")
+
+@cppcheck.checker
+def EmptySwitchStatement(cfg, data):
+    for token in cfg.tokenlist:
+        if not match(token, "switch (*) { }"):
+            continue
+        cppcheck.reportError(token, "style", "Empty switch statement.")
+
+@cppcheck.checker
+def EmptyWhileStatement(cfg, data):
+    for token in cfg.tokenlist:
+        if not match(token, "while (*) { }"):
+            continue
+        cppcheck.reportError(token, "style", "Empty while statement.")
+
+@cppcheck.checker
+def ForLoopShouldBeWhileLoop(cfg, data):
+    for token in cfg.tokenlist:
+        if not match(token, "for ( ; !!;"):
+            continue
+        # Skip empty for loops
+        if match(token, "for (*) { }"):
+            continue
+        end = token.next.link
+        if not match(end.tokAt(-1), "; )"):
+            continue
+        cppcheck.reportError(token, "style", "For loop should be written as a while loop.")
+
+@cppcheck.checker
+def GotoStatement(cfg, data):
+    for token in cfg.tokenlist:
+        if not match(token, "goto"):
+            continue
+        cppcheck.reportError(token, "style", "Goto considered harmful.")
+
+# @cppcheck.checker
+# def InvertedLogic(cfg, data):
+#     for token in cfg.tokenlist:
+#         cond = None
+#         if match(token, "if (*) {*} else { !!if"):
+#             cond = token.next.astOperand2
+#         elif match(token, "?"):
+#             cond = token.astOperand1
+#         if not match(cond, "!|!="):
+#             continue
+#         cppcheck.reportError(cond, "style", "It is cleaner to invert the logic.")
+
+@cppcheck.checker
+def LambdaAttribute(cfg, data):
+    for token in cfg.tokenlist:
+        if not match(token, "] __device__|__host__ {|{}}"):
+            continue
+        cppcheck.reportError(token, "style", "Attributes to lambdas must come after parameter list.")
+
+@cppcheck.checker
+def MultipleUnaryOperator(cfg, data):
+    for token in cfg.tokenlist:
+        if not match(token, "+|-|~|!"):
+            continue
+        if not token.isUnaryOp(token.str):
+            continue
+        if not match(token.astOperand1, "+|-|~|!"):
+            continue
+        if not token.astOperand1.isUnaryOp(token.astOperand1.str):
+            continue
+        cppcheck.reportError(token, "style", "Muliple unary operators used together.")
+
+@cppcheck.checker
+def MutableVariable(cfg, data):
+    for token in cfg.tokenlist:
+        if not match(token, "mutable %var%"):
+            continue
+        cppcheck.reportError(token, "style", "Do not create mutable variables.")
+
+@cppcheck.checker
+def NestedBlocks(cfg, data):
+    for token in cfg.tokenlist:
+        block = match(token, "if|while|for|switch (*) { {*}@block }").block
+        if not block:
+            block = match(token, "; { {*}@block break ; }").block
+        if not block:
+            continue
+        cppcheck.reportError(block, "style", "Block directly inside block.")
+
+@cppcheck.checker
+def RedundantCast(cfg, data):
+    for token in cfg.tokenlist:
+        m = match(token, "%var%@decl ; %var%@assign = static_cast <*>@cast (*) ;")
+        if not m:
+            continue
+        if m.decl.varId != m.assign.varId:
+            continue
+        if not match(token.previous, "auto"):
+            if not isTokensEqual(getVariableDecl(m.decl.variable), getInnerLink(m.cast), skip='const|volatile|&|&&|*'):
+                continue
+        if not match(token, "%var%@decl ; %var%@assign = static_cast <*>@cast (*) ;"):
+            continue
+        cppcheck.reportError(token, "style", "Static cast is redundant.")
+
+@cppcheck.checker
+def RedundantConditionalOperator(cfg, data):
+    for token in cfg.tokenlist:
+        if not match(token, "? true|false : true|false"):
+            continue
+        cppcheck.reportError(token, "style", "Conditional operator is redundant.")
+
+@cppcheck.checker
+def RedundantIfStatement(cfg, data):
+    for token in cfg.tokenlist:
+        if not match(token, "if (*) { return true|false ; } else { return true|false ; }"):
+            continue
+        cppcheck.reportError(token, "style", "The if statement is redundant.")
+
+@cppcheck.checker
+def RedundantLocalVariable(cfg, data):
+    for token in cfg.tokenlist:
+        m = match(token, "%var%@decl ; %var%@assign = **; return %var%@returned ;")
+        if not m:
+            continue
+        if m.decl.varId != m.assign.varId:
+            continue
+        if m.decl.varId != m.returned.varId:
+            continue
+        cppcheck.reportError(m.returned, "style", "Variable is returned immediately after its declaration, can be simplified to just return expression.")
+
+# @cppcheck.checker
+# def UnnecessaryElseStatement(cfg, data):
+#     for token in cfg.tokenlist:
+#         m = match(token, "if (*) {*}@block else@else_statement {")
+#         if not m:
+#             continue
+#         stmt = m.block.link.tokAt(-2)
+#         if not match(stmt, "%any% ; }"):
+#             continue
+#         if not match(stmt, "break|continue"):
+#             stmt = stmt.astTop()
+#         if not match(stmt, "break|continue|return|throw"):
+#             continue
+#         cppcheck.reportError(m.else_statement, "style", "Else statement is not necessary.")
+
+@cppcheck.checker
+def UnnecessaryEmptyCondition(cfg, data):
+    for token in cfg.tokenlist:
+        m = match(token, "if (*)@if_cond { for (*)@for_cond {*} }")
+        if not m:
+            continue
+        cond = m.if_cond.astOperand2
+        if match(cond, "!"):
+            cond = cond.astOperand1
+        if not match(cond.tokAt(-2), ". empty ("):
+            continue
+        container = cond.tokAt(-2).astOperand1
+        if not container.varId:
+            continue
+        if not match(m.for_cond.astOperand2, ":"):
+            continue
+        container_iter = m.for_cond.astOperand2.astOperand2
+        if container_iter.varId != container.varId:
+            continue
+        cppcheck.reportError(container, "style", "Unnecessary check for empty before for range loop.")
+
+@cppcheck.checker
+def UseDeviceLaunch(cfg, data):
+    for token in cfg.tokenlist:
+        if not match(token, "hipLaunchKernelGGL ("):
+            continue
+        cppcheck.reportError(token, "style", "Use device::launch instead.")
+
+@cppcheck.checker
+def UseManagePointer(cfg, data):
+    for token in cfg.tokenlist:
+        if not match(token, "fclose|free|hipFree|hipHostFree|hipFreeArray|hipMemFree|hipStreamDestroy|hipEventDestroy|hipArrayDestroy|hipCtxDestroy|hipDestroyTextureObject|hipDestroySurfaceObject|miirDestroyHandle ("):
+            continue
+        cppcheck.reportError(token, "style", "Use manage pointer for resource management.")
+
+@cppcheck.checker
+def UseSmartPointer(cfg, data):
+    for token in cfg.tokenlist:
+        if not match(token, "new %name%"):
+            continue
+        cppcheck.reportError(token, "style", "Use manage pointer for resource management.")
+
+@cppcheck.checker
+def useStlAlgorithms(cfg, data):
+    for token in cfg.tokenlist:
+        if match(token, "memcpy|strcpy|strncpy|strcat|strncat ("):
+            cppcheck.reportError(token, "style", "Use std::copy instead.")
+        elif match(token, "memset ("):
+            cppcheck.reportError(token, "style", "Use std::fill instead.")
+        elif match(token, "memcmp ("):
+            cppcheck.reportError(token, "style", "Use std::equal_range instead.")
+        elif match(token, "memchr ("):
+            cppcheck.reportError(token, "style", "Use std::find instead.")
+

cppcheck.rules → tools/cppcheck/rules.xml

@@ -44,69 +44,6 @@
         <summary>Macros must be prefixed with MIGRAPHX_</summary>
     </message>
 </rule>
-<rule>
-    <tokenlist>normal</tokenlist>
-    <pattern>mutable \w+</pattern>
-    <message>
-        <id>MutableVariable</id>
-        <severity>style</severity>
-        <summary>Do not create mutable variables.</summary>
-    </message>
-</rule>
-<rule>
-    <tokenlist>normal</tokenlist>
-    <pattern>(memcpy|strcpy|strncpy|strcat|strncat) \(</pattern>
-    <message>
-        <id>useStlAlgorithms</id>
-        <severity>style</severity>
-        <summary>Use std::copy instead</summary>
-    </message>
-</rule>
-<rule>
-    <tokenlist>normal</tokenlist>
-    <pattern>memset \(</pattern>
-    <message>
-        <id>useStlAlgorithms</id>
-        <severity>style</severity>
-        <summary>Use std::fill instead</summary>
-    </message>
-</rule>
-<rule>
-    <tokenlist>normal</tokenlist>
-    <pattern>memcmp \(</pattern>
-    <message>
-        <id>useStlAlgorithms</id>
-        <severity>style</severity>
-        <summary>Use std::equal_range instead</summary>
-    </message>
-</rule>
-<rule>
-    <tokenlist>normal</tokenlist>
-    <pattern>memchr \(</pattern>
-    <message>
-        <id>useStlAlgorithms</id>
-        <severity>style</severity>
-        <summary>Use std::find instead</summary>
-    </message>
-</rule>
-<rule>
-    <tokenlist>normal</tokenlist>
-    <pattern>\\W(fclose|free|hipFree|hipHostFree|hipFreeArray|hipMemFree|hipStreamDestroy|hipEventDestroy|hipArrayDestroy|hipCtxDestroy|hipDestroyTextureObject|hipDestroySurfaceObject|miirDestroyHandle) \(</pattern>
-    <message>
-        <id>useManagePointer</id>
-        <severity>style</severity>
-        <summary>Use manage pointer for resource management</summary>
-    </message>
-</rule>
-<rule>
-    <tokenlist>normal</tokenlist>
-    <pattern><![CDATA[new \w+]]></pattern>
-    <message>
-        <id>useSmartPointer</id>
-        <severity>style</severity>
-        <summary>Use make_shared or make_unique instead of new</summary>
-    </message>
-</rule>
 <rule>
     <tokenlist>raw</tokenlist>
     <pattern><![CDATA[ [^\(,;{}:]+ \w+ && (\w+|\(|\+|\-|\*)]]></pattern>
@@ -143,24 +80,6 @@
         <summary>Use 'not' instead of !</summary>
     </message>
 </rule>
-<!-- <rule>
-    <tokenlist>raw</tokenlist>
-    <pattern><![CDATA[] (__device__ |__host__ )+(\(|{)]]></pattern>
-    <message>
-        <id>LambdaAttribute</id>
-        <severity>warning</severity>
-        <summary>Attributes to lambdas must come after parameter list.</summary>
-    </message>
-</rule> -->
-<rule>
-    <tokenlist>normal</tokenlist>
-    <pattern><![CDATA[hipLaunchKernelGGL \( (?!\( \w+ < \w+ > \))]]></pattern>
-    <message>
-        <id>UseDeviceLaunch</id>
-        <severity>style</severity>
-        <summary>Use device::launch instead</summary>
-    </message>
-</rule>
 <rule>
     <tokenlist>normal</tokenlist>
     <pattern><![CDATA[if (\([^()]*(?-1)*[^()]*\)) { [^{}]* (return|throw|break|continue) [^;]* ; } else {]]></pattern>
@@ -170,150 +89,6 @@
         <summary>Else statement is not necessary.</summary>
     </message>
 </rule>
-<rule>
-    <tokenlist>normal</tokenlist>
-    <pattern><![CDATA[((?:(?:\w+|<|>|::) )*(?:\w+|>)(?: &|\*)*) (\w+) ; \2 = static_cast < \1 > (\([^()]*(?-1)*[^()]*\)) ;]]></pattern>
-    <message>
-        <id>RedundantCast</id>
-        <severity>style</severity>
-        <summary>Static cast is redundant.</summary>
-    </message>
-</rule>
-<rule>
-    <tokenlist>normal</tokenlist>
-    <pattern><![CDATA[auto (\w+) ; \1 = static_cast < (?:(?:\w+|<|>|::) )*(?:\w+|>)(?: &|\*)* > (\([^()]*(?-1)*[^()]*\)) ;]]></pattern>
-    <message>
-        <id>RedundantCast</id>
-        <severity>style</severity>
-        <summary>Static cast is redundant.</summary>
-    </message>
-</rule>
-<rule>
-    <tokenlist>normal</tokenlist>
-    <pattern><![CDATA[\? (true|false) : (true|false)]]></pattern>
-    <message>
-        <id>RedundantConditionalOperator</id>
-        <severity>style</severity>
-        <summary>Conditional operator is redundant.</summary>
-    </message>
-</rule>
-<rule>
-    <tokenlist>normal</tokenlist>
-    <pattern><![CDATA[switch (\([^()]*(?-1)*[^()]*\)) { }]]></pattern>
-    <message>
-        <id>EmptySwitchStatement</id>
-        <severity>style</severity>
-        <summary>Empty switch statement.</summary>
-    </message>
-</rule>
-<rule>
-    <tokenlist>normal</tokenlist>
-    <pattern><![CDATA[(?:(?:\w+|<|>|::) )*(?:\w+|>)(?: &|\*)* (\w) ; \1 = [^;]+ ; return \1 ;]]></pattern>
-    <message>
-        <id>RedundantLocalVariable</id>
-        <severity>style</severity>
-        <summary>Variable is returned immediately after its declaration, can be simplified to just return expression.</summary>
-    </message>
-</rule>
-<rule>
-    <tokenlist>normal</tokenlist>
-    <pattern><![CDATA[for \( ; [^;]+ ; \)]]></pattern>
-    <message>
-        <id>ForLoopShouldBeWhileLoop</id>
-        <severity>style</severity>
-        <summary>For loop should be written as a while loop.</summary>
-    </message>
-</rule>
-<rule>
-    <tokenlist>normal</tokenlist>
-    <pattern><![CDATA[while (\([^()]*(?-1)*[^()]*\)) { }]]></pattern>
-    <message>
-        <id>EmptyWhileStatement</id>
-        <severity>style</severity>
-        <summary>Empty while statement.</summary>
-    </message>
-</rule>
-<rule>
-    <tokenlist>normal</tokenlist>
-    <pattern><![CDATA[if \( \w+ (\||&) \w+ \)]]></pattern>
-    <message>
-        <id>BitwiseOperatorInConditional</id>
-        <severity>style</severity>
-        <summary>Bitwise operator found in if statement.</summary>
-    </message>
-</rule>
-<rule>
-    <tokenlist>normal</tokenlist>
-    <pattern><![CDATA[else { }]]></pattern>
-    <message>
-        <id>EmptyElseBlock</id>
-        <severity>style</severity>
-        <summary>Empty else statement can be safely removed.</summary>
-    </message>
-</rule>
-<rule>
-    <tokenlist>normal</tokenlist>
-    <pattern><![CDATA[for (\([^()]*(?-1)*[^()]*\)) { }]]></pattern>
-    <message>
-        <id>EmptyForStatement</id>
-        <severity>style</severity>
-        <summary>Empty for statement.</summary>
-    </message>
-</rule>
-<rule>
-    <tokenlist>normal</tokenlist>
-    <pattern><![CDATA[if (\([^()]*(?-1)*[^()]*\)) { }]]></pattern>
-    <message>
-        <id>EmptyIfStatement</id>
-        <severity>style</severity>
-        <summary>Empty if statement.</summary>
-    </message>
-</rule>
-<rule>
-    <tokenlist>normal</tokenlist>
-    <pattern><![CDATA[if (\([^()]*(?-1)*[^()]*\)) { return (true|false) ; } else { return (true|false) ; }]]></pattern>
-    <message>
-        <id>RedundantIfStatement</id>
-        <severity>style</severity>
-        <summary>The if statement is redundant.</summary>
-    </message>
-</rule>
-<rule>
-    <tokenlist>normal</tokenlist>
-    <pattern><![CDATA[! !]]></pattern>
-    <message>
-        <id>DoubleNegative</id>
-        <severity>style</severity>
-        <summary>Double negative is always positive.</summary>
-    </message>
-</rule>
-<rule>
-    <tokenlist>normal</tokenlist>
-    <pattern><![CDATA[~ ~]]></pattern>
-    <message>
-        <id>DoubleNegative</id>
-        <severity>style</severity>
-        <summary>Double negative is always positive.</summary>
-    </message>
-</rule>
-<rule>
-    <tokenlist>normal</tokenlist>
-    <pattern><![CDATA[! \( !]]></pattern>
-    <message>
-        <id>DoubleNegative</id>
-        <severity>style</severity>
-        <summary>Double negative is always positive.</summary>
-    </message>
-</rule>
-<rule>
-    <tokenlist>normal</tokenlist>
-    <pattern><![CDATA[~ \( ~]]></pattern>
-    <message>
-        <id>DoubleNegative</id>
-        <severity>style</severity>
-        <summary>Double negative is always positive.</summary>
-    </message>
-</rule>
 <rule>
     <tokenlist>normal</tokenlist>
     <pattern><![CDATA[if \( \w+ != \w+ \) ({[^{}]*(?-1)*[^{}]*}) else { (?!if)]]></pattern>
@@ -350,51 +125,6 @@
         <summary>It is cleaner to invert the logic.</summary>
     </message>
 </rule>
-<rule>
-    <tokenlist>normal</tokenlist>
-    <pattern><![CDATA[catch (\([^()]*(?-1)*[^()]*\)) { }]]></pattern>
-    <message>
-        <id>EmptyCatchStatement</id>
-        <severity>style</severity>
-        <summary>An empty catch statement.</summary>
-    </message>
-</rule>
-<rule>
-    <tokenlist>normal</tokenlist>
-    <pattern><![CDATA[if (\([^()]*(?-1)*[^()]*\)) { assert (\([^()]*(?-1)*[^()]*\)) ; }]]></pattern>
-    <message>
-        <id>ConditionalAssert</id>
-        <severity>style</severity>
-        <summary>The if condition should be included in assert.</summary>
-    </message>
-</rule>
-<rule>
-    <tokenlist>normal</tokenlist>
-    <pattern><![CDATA[if \( (\w) . empty \( \) \) { for \( (?:(?:\w+|<|>|::) )*(?:\w+|>)(?: &|\*)* \w : \1 \) ({[^{}]*(?-1)*[^{}]*}) }]]></pattern>
-    <message>
-        <id>UnnecessaryEmptyCondition</id>
-        <severity>style</severity>
-        <summary>Unnecessary check for empty before for range loop.</summary>
-    </message>
-</rule>
-<rule>
-    <tokenlist>normal</tokenlist>
-    <pattern><![CDATA[if \( ! (\w) . empty \( \) \) { for \( (?:(?:\w+|<|>|::) )*(?:\w+|>)(?: &|\*)* \w : \1 \) ({[^{}]*(?-1)*[^{}]*}) }]]></pattern>
-    <message>
-        <id>UnnecessaryEmptyCondition</id>
-        <severity>style</severity>
-        <summary>Unnecessary check for empty before for range loop.</summary>
-    </message>
-</rule>
-<rule>
-    <tokenlist>normal</tokenlist>
-    <pattern><![CDATA[for \( (?:(?:\w+|<|>|::) )*(?:\w+|>)(?: &|\*)* (\w+) = \w+ ; \1 < \w+ ; (\1 \+\+|\+\+ \1|\1 \-\-|\-\- \1) \) { \w+ \[ \1 \] = \w+ \[ \1 \] ; }]]></pattern>
-    <message>
-        <id>useStlAlgorithm</id>
-        <severity>style</severity>
-        <summary>Considering using std::copy instead.</summary>
-    </message>
-</rule>
 <rule>
     <tokenlist>normal</tokenlist>
     <pattern><![CDATA[for \( (?:(?:\w+|<|>|::) )*(?:\w+|>)(?: &|\*)* (\w+) = \w+ ; \1 < \w+ ; (\1 \+\+|\+\+ \1|\1 \-\-|\-\- \1) \) { \w+ \[ \1 \] = \w+ ; }]]></pattern>
@@ -467,21 +197,3 @@
         <summary>Considering using std::find or std::find_if instead.</summary>
     </message>
 </rule>
-<rule>
-    <tokenlist>normal</tokenlist>
-    <pattern><![CDATA[do { } while \(]]></pattern>
-    <message>
-        <id>EmptyDoWhileStatement</id>
-        <severity>style</severity>
-        <summary>Empty do-while.</summary>
-    </message>
-</rule>
-<rule>
-    <tokenlist>normal</tokenlist>
-    <pattern>goto</pattern>
-    <message>
-        <id>GotoStatement</id>
-        <severity>style</severity>
-        <summary>Goto considered harmful.</summary>
-    </message>
-</rule>