Address feedback

Author: MichaelRFairhurst

c/common/src/codingstandards/c/Generic.qll

@@ -115,7 +115,7 @@ class ParsedGenericMacro extends Macro {
     )
   }
 
-  string getControllingExprString() { result = getSelectionString(1) }
+  string getControllingExprString() { result = getSelectionString(1).trim() }
 
   bindingset[str, word]
   private int countWordInString(string word, string str) {

c/misra/src/rules/RULE-23-1/GenericSelectionDoesntDependOnMacroArgument.ql

@@ -19,8 +19,9 @@ import codingstandards.c.Generic
 from ParsedGenericMacro macro, string ctrlExpr
 where
   not isExcluded(macro, GenericsPackage::genericSelectionDoesntDependOnMacroArgumentQuery()) and
-  ctrlExpr = macro.getControllingExprString().trim() and
-  not macro.expansionsInsideControllingExpr(_) > 0
+  ctrlExpr = macro.getControllingExprString() and
+  // No parameter exists that is expanded in the controlling expression one or more times
+  not exists(string parameter | macro.expansionsInsideControllingExpr(parameter) > 0)
 select macro,
-  "Generic macro " + macro.getName() + " uses controlling expr " + ctrlExpr +
-    ", which doesn't match any macro parameter."
+  "Generic macro " + macro.getName() + " doesn't refer to a macro parameter in controlling expr '" +
+    ctrlExpr + "'."

c/misra/src/rules/RULE-23-1/GenericSelectionNotExpandedFromAMacro.ql

@@ -19,5 +19,5 @@ where
   not isExcluded(generic, GenericsPackage::genericSelectionNotExpandedFromAMacroQuery()) and
   ctrlExpr = generic.getControllingExpr() and
   not exists(MacroInvocation mi | mi.getAGeneratedElement() = generic.getExpr())
-select generic, "Generic expression with controlling expression $@ is not expanded froma macro",
-  ctrlExpr, ctrlExpr.toString()
+select generic, "$@ in generic expression does not expand a macro parameter.", ctrlExpr,
+  "Controlling expression"

c/misra/src/rules/RULE-23-2/GenericSelectionNotFromMacroWithSideEffects.ql

@@ -72,6 +72,7 @@ module GenericSideEffectReportConfig implements MacroReportConfigSig<GenericWith
   string getMessageNotInMacro(
     GenericWithNonMacroSideEffect element, Locatable optLoc1, string optStr1
   ) {
+    // Generics which are not expanded from a macro aren't applicable to this rule.
     none()
   }
 }

c/misra/src/rules/RULE-23-3/GenericWithoutNonDefaultAssociation.ql

@@ -25,7 +25,7 @@ class InvalidGeneric extends C11GenericExpr {
   }
 }
 
-from C11GenericExpr generic, Element primaryElement
+from InvalidGeneric generic, Element primaryElement
 where
   not isExcluded(primaryElement, GenericsPackage::genericWithoutNonDefaultAssociationQuery()) and
   not exists(Type t |

c/misra/test/rules/RULE-23-1/GenericSelectionDoesntDependOnMacroArgument.expected

@@ -1,3 +1,3 @@
-| test.c:2:1:2:31 | #define M1 _Generic(1, int : 1) | Generic macro M1 uses controlling expr 1, which doesn't match any macro parameter. |
-| test.c:4:1:4:34 | #define M2(X) _Generic(1, int : X) | Generic macro M2 uses controlling expr 1, which doesn't match any macro parameter. |
-| test.c:18:1:18:39 | #define M9(X) g(_Generic((Y), int : 1)) | Generic macro M9 uses controlling expr (Y), which doesn't match any macro parameter. |
+| test.c:2:1:2:31 | #define M1 _Generic(1, int : 1) | Generic macro M1 doesn't refer to a macro parameter in controlling expr '1'. |
+| test.c:4:1:4:34 | #define M2(X) _Generic(1, int : X) | Generic macro M2 doesn't refer to a macro parameter in controlling expr '1'. |
+| test.c:18:1:18:39 | #define M9(X) g(_Generic((Y), int : 1)) | Generic macro M9 doesn't refer to a macro parameter in controlling expr '(Y)'. |

c/misra/test/rules/RULE-23-1/GenericSelectionNotExpandedFromAMacro.expected

@@ -1 +1 @@
-| test.c:21:3:21:22 | _Generic | Generic expression with controlling expression $@ is not expanded froma macro | test.c:21:12:21:12 | 1 | 1 |
+| test.c:21:3:21:22 | _Generic | $@ in generic expression does not expand a macro parameter. | test.c:21:12:21:12 | 1 | Controlling expression |

cpp/common/src/codingstandards/cpp/types/Compatible.qll

@@ -369,53 +369,4 @@ private class FunctionType extends Type {
     result = this.(RoutineType).getParameterType(i) or
     result = this.(FunctionPointerIshType).getParameterType(i)
   }
-}
-/*
- * predicate typesCompatibleImpl(Type t1, Type t2) {
- *  // A type is compatible with itself
- *  t1 = t2
- *  or
- *  // All specifiers must match, but the order does not matter:
- *  (
- *    t1 instanceof SpecifiedType and
- *    t2 instanceof SpecifiedType
- *  ) and
- *  specifiersMatchExactly(t1, t2) and
- *  typesCompatibleImpl(t1.stripTopLevelSpecifiers(), t2.stripTopLevelSpecifiers())
- *  or
- *  // Identically qualified pointers are compatible if they point to compatible types.
- *  typesCompatibleImpl(t1.(PointerType).getBaseType(), t2.(PointerType).getBaseType())
- *  or
- *  // Array objects are compatible if they have a compatible element type. If both have a constant
- *  // size then that size must match.
- *  typesCompatibleImpl(t1.(ArrayType).getBaseType(), t2.(ArrayType).getBaseType()) and
- *  count(int i | i = [t1, t2].(ArrayType).getSize()) < 2
- *  or
- *  // Enum types are compatible with one of char, int, or signed int, but the implementation
- *  // decides.
- *  [t1, t2] instanceof Enum and
- *  ([t1, t2] instanceof CharType or [t1, t2] instanceof IntType)
- *  or
- *  // `int` is the same as `signed`, `signed int`, while `unsigned` is the same as `unsigned int`.
- *  t1.(IntegralType).getCanonicalArithmeticType() = t2.(IntegralType).getCanonicalArithmeticType()
- *  or
- *  // Function types are compatible if they have the same return type and compatible parameters.
- *  // Technically, variadic functions are have special behavior not covered here.
- *  exists(RoutineType f1, RoutineType f2 | f1 = t1 and f2 = t2 |
- *    typesCompatibleImpl(f1.getReturnType(), f2.getReturnType()) and
- *    forall(int i | exists([f1, f2].getParameterType(i)) |
- *      typesCompatibleImpl(f1.getParameterType(i), f2.getParameterType(i))
- *    )
- *  )
- *  or
- *  // Function pointer types should be covered by `PointerType` and `RoutineType` above, but that is
- *  // not how they are implemented in CodeQL.
- *  exists(FunctionPointerIshType f1, FunctionPointerIshType f2 | f1 = t1 and f2 = t2 |
- *    typesCompatibleImpl(f1.getReturnType(), f2.getReturnType()) and
- *    forall(int i | exists([f1, f2].getParameterType(i)) |
- *      typesCompatibleImpl(f1.getParameterType(i), f2.getParameterType(i))
- *    )
- *  )
- * }
- */
-
+}

cpp/common/src/codingstandards/cpp/types/SimpleAssignment.qll

@@ -1,3 +1,9 @@
+/**
+ * Helper predicates related to C11/C17 constraints on simple assignment between two types.
+ * 
+ * Currently only a subset of the constraints are implemented, specifically those
+ * related to pointer types.
+ */
 import codingstandards.cpp.types.LvalueConversion
 import codingstandards.cpp.types.Compatible