Preprocessor6: address review comments DIR-4-9

Author: knewbury01

c/misra/src/rules/DIR-4-9/FunctionOverFunctionLikeMacro.ql

@@ -18,37 +18,47 @@ import codingstandards.c.misra
 import codingstandards.cpp.FunctionLikeMacro
 import codingstandards.cpp.Naming
 
-predicate omission(Macro i) { Naming::Cpp14::hasStandardLibraryMacroName(i.getName()) }
-
 abstract class IrreplaceableFunctionLikeMacro extends FunctionLikeMacro { }
 
+/** A standard library function like macro that contains the use of a stringize or tokenize operator should not be replaced by a function. */
+private class StringizeOrTokenizeMacro extends IrreplaceableFunctionLikeMacro {
+  StringizeOrTokenizeMacro() { this.getBody().regexpMatch(".*\\#{1,2}?.*") }
+}
+
+/** A standard library function like macro that should not be replaced by a function. */
+private class StandardLibraryFunctionLikeMacro extends IrreplaceableFunctionLikeMacro {
+  StandardLibraryFunctionLikeMacro() { Naming::Cpp14::hasStandardLibraryMacroName(this.getName()) }
+}
+
+/** A function like macro invocation as an `asm` argument cannot be replaced by a function. */
 private class AsmArgumentInvoked extends IrreplaceableFunctionLikeMacro {
   AsmArgumentInvoked() {
     any(AsmStmt s).getLocation().subsumes(this.getAnInvocation().getLocation())
   }
 }
 
+/** A macro that is only invoked with constant arguments is more likely to be compile-time evaluated than a function call so do not suggest replacement. */
 private class OnlyConstantNumericInvoked extends IrreplaceableFunctionLikeMacro {
   OnlyConstantNumericInvoked() {
     forex(MacroInvocation mi | mi = this.getAnInvocation() |
+      //int/float literals
       mi.getUnexpandedArgument(_).regexpMatch("\\d+")
+      or
+      //char/string literal
+      mi.getUnexpandedArgument(_).regexpMatch("(\\'|\")+.*")
     )
   }
 }
 
-private class KnownIrreplaceableFunctionLikeMacro extends IrreplaceableFunctionLikeMacro {
-  KnownIrreplaceableFunctionLikeMacro() {
-    this.getName() in ["UNUSED", "__has_builtin", "MIN", "MAX"]
-  }
-}
-
+/** A function like macro invoked to initialize an object with static storage that cannot be replaced with a function call. */
 private class UsedToStaticInitialize extends IrreplaceableFunctionLikeMacro {
   UsedToStaticInitialize() {
     any(StaticStorageDurationVariable v).getInitializer().getExpr() =
       this.getAnInvocation().getExpr()
   }
 }
 
+/** A function like macro that is called with an argument that is an operator that cannot be replaced with a function call. */
 private class FunctionLikeMacroWithOperatorArgument extends IrreplaceableFunctionLikeMacro {
   FunctionLikeMacroWithOperatorArgument() {
     exists(MacroInvocation mi | mi.getMacro() = this |
@@ -57,32 +67,6 @@ private class FunctionLikeMacroWithOperatorArgument extends IrreplaceableFunctio
   }
 }
 
-abstract class UnsafeMacro extends FunctionLikeMacro { }
-
-class ParameterNotUsedMacro extends UnsafeMacro {
-  ParameterNotUsedMacro() {
-    //parameter not used - has false positives on args that are not used but are substrings of other args
-    exists(string p |
-      p = this.getAParameter() and
-      not this.getBody().regexpMatch(".*(\\s*|\\(|\\)|\\##)" + p + "(\\s*||\\)|\\(|\\##).*")
-    )
-  }
-}
-
-class ParameterMoreThanOnceMacro extends UnsafeMacro {
-  ParameterMoreThanOnceMacro() {
-    //parameter used more than once
-    exists(string p |
-      p = this.getAParameter() and
-      exists(int i, string newstr |
-        newstr = this.getBody().replaceAll(p, "") and
-        i = ((this.getBody().length() - newstr.length()) / p.length()) and
-        i > 1
-      )
-    )
-  }
-}
-
 predicate partOfConstantExpr(MacroInvocation i) {
   exists(Expr e |
     e.isConstant() and
@@ -94,8 +78,6 @@ predicate partOfConstantExpr(MacroInvocation i) {
 from FunctionLikeMacro m
 where
   not isExcluded(m, Preprocessor6Package::functionOverFunctionLikeMacroQuery()) and
-  not omission(m) and
-  m instanceof UnsafeMacro and
   not m instanceof IrreplaceableFunctionLikeMacro and
   //function call not allowed in a constant expression (where constant expr is parent)
   forall(MacroInvocation i | i = m.getAnInvocation() | not partOfConstantExpr(i))

c/misra/test/rules/DIR-4-9/FunctionOverFunctionLikeMacro.expected

@@ -1 +1 @@
-| test.c:3:1:3:36 | #define MACRO3(L,R) (L " " R " " L) | Macro used when function call would be preferred. |
+| test.c:4:1:6:12 | #define MACRO4(L) (L" " "suffix") | Macro used when function call would be preferred. |

c/misra/test/rules/DIR-4-9/test.c

@@ -1,26 +1,30 @@
-#define MACRO(OP, L, R) ((L)OP(R))
-#define MACRO2(L, R) (L + R)
-#define MACRO3(L, R) (L " " R " " L)
+#define MACRO(OP, L, R) ((L)OP(R))   // COMPLIANT
+#define MACRO2(L, R) (L + R)         // COMPLIANT
+#define MACRO3(L, R) (L " " R " " L) // COMPLIANT
 #define MACRO4(L)                                                              \
   (L" "                                                                        \
-   "suffix")
-#define MACRO5(L, LR) (LR + 1)
-#define MACRO6(X, LR) (LR + 1)
+   "suffix")                   // NON_COMPLIANT
+#define MACRO5(L, LR) (LR + 1) // COMPLIANT
+#define MACRO6(X, LR) (LR + 1) // COMPLIANT
+#define MACRO7(x, y) x##y      // COMPLIANT
 
-const char a1[MACRO2(1, 1) + 6]; // COMPLIANT
+const char a1[MACRO2(1, 1) + 6];
 
 void f() {
-  int i = MACRO(+, 1, 1); // COMPLIANT
+  int i = MACRO(+, 1, 1);
+  int i2 = MACRO2(7, 10);
 
-  int i2 = MACRO2(7, 10); // COMPLIANT
+  static int i3 = MACRO2(1, 1);
 
-  static int i3 = MACRO2(1, 1); // COMPLIANT
+  char *i4 = MACRO3("prefix", "suffix");
 
-  char *i4 = MACRO3("prefix", "suffix"); // NON_COMPLIANT
+  char *i5 = MACRO4("prefix");
 
-  char *i5 = MACRO4("prefix"); // COMPLIANT
+  char *i6 = MACRO4(MACRO2(1, 1));
 
-  int i6 = MACRO5(1, 1); // COMPLIANT
+  int i7 = MACRO5(1, 1);
 
-  int i7 = MACRO6(1, 1); // COMPLIANT
+  int i8 = MACRO6(1, 1);
+
+  char *i9 = MACRO7("prefix", "suffix");
 }