Merge pull request #551 from kraiouchkine/a8-4-7-fix-429

A8-4-7: Consider non-trivially-copyable types

Author: knewbury01

change_notes/2024-03-04-improve-a8-4-7.md

@@ -0,0 +1,3 @@
+- `A8-4-7` - `InParametersForNotCheapToCopyTypesNotPassedByReference.ql`, `InParametersForCheapToCopyTypesNotPassedByValue.ql`:
+  - Improve coverage of the query by additionally alerting to non-trivially-copyable types being passed by value.
+  - Non-trivially-copyable types not passed by value will no longer be incorrectly reported.

cpp/autosar/src/rules/A8-4-7/InParametersForCheapToCopyTypesNotPassedByValue.ql

@@ -15,7 +15,7 @@
 
 import cpp
 import codingstandards.cpp.autosar
-import TriviallySmallType
+import TriviallyCopyableSmallType
 import codingstandards.cpp.CommonTypes as CommonTypes
 import codingstandards.cpp.Class
 
@@ -26,7 +26,7 @@ import codingstandards.cpp.Class
  * In this rule, we will look cases where a "cheap to copy" type is not passed by value.
  */
 
-from Parameter v, TriviallySmallType t
+from Parameter v, TriviallyCopyableSmallType t
 where
   not isExcluded(v, ClassesPackage::inParametersForCheapToCopyTypesNotPassedByValueQuery()) and
   exists(ReferenceType rt |
@@ -40,5 +40,6 @@ where
   not v.isFromUninstantiatedTemplate(_) and
   not v.isFromTemplateInstantiation(_)
 select v,
-  "Parameter '" + v.getName() + "' is the trivially copyable type '" + t.getName() +
-    "' but it is passed by reference instead of by value."
+  "Parameter '" + v.getName() +
+    "' is the trivially copyable type $@ but it is passed by reference instead of by value.", t,
+  t.getName()

cpp/autosar/src/rules/A8-4-7/InParametersForNotCheapToCopyTypesNotPassedByReference.ql

@@ -15,7 +15,7 @@
 
 import cpp
 import codingstandards.cpp.autosar
-import TriviallySmallType
+import TriviallyCopyableSmallType
 import codingstandards.cpp.CommonTypes as CommonTypes
 
 /*
@@ -28,12 +28,12 @@ import codingstandards.cpp.CommonTypes as CommonTypes
 from Parameter v
 where
   not isExcluded(v, ClassesPackage::inParametersForNotCheapToCopyTypesNotPassedByReferenceQuery()) and
-  not v.getType() instanceof TriviallySmallType and
+  not v.getType() instanceof TriviallyCopyableSmallType and
   not v.getType().getUnderlyingType() instanceof ReferenceType and
   not exists(CatchBlock cb | cb.getParameter() = v) and
   not v.isFromUninstantiatedTemplate(_) and
   not v.isFromTemplateInstantiation(_)
 select v,
-  "Parameter " + v.getName() +
-    " is the trivially non-copyable type $@ but it is passed by value instead of by reference.",
+  "Parameter '" + v.getName() +
+    "' is the trivially non-copyable type $@ but it is passed by value instead of by reference.",
   v.getType(), v.getType().getName()

cpp/autosar/src/rules/A8-4-7/TriviallySmallType.qll renamed to cpp/autosar/src/rules/A8-4-7/TriviallyCopyableSmallType.qll

@@ -1,5 +1,6 @@
 import cpp
 import codingstandards.cpp.autosar
+import codingstandards.cpp.TrivialType
 
 /**
  * Get the largest word size, in bytes. Some projects may have multiple different
@@ -13,6 +14,9 @@ int wordSize() { result = max(VoidPointerType v | | v.getSize()) }
 bindingset[bytes]
 int bytesToWords(int bytes) { result = bytes / wordSize() }
 
-class TriviallySmallType extends Type {
-  TriviallySmallType() { exists(int size | size = this.getSize() | bytesToWords(size) <= 2) }
+class TriviallyCopyableSmallType extends Type {
+  TriviallyCopyableSmallType() {
+    isTriviallyCopyableType(this) and
+    exists(int size | size = this.getSize() | bytesToWords(size) <= 2)
+  }
 }

cpp/autosar/test/rules/A8-4-7/InParametersForCheapToCopyTypesNotPassedByValue.expected

@@ -1 +1 @@
-| test.cpp:20:19:20:21 | f5a | Parameter 'f5a' is the trivially copyable type 'const S1' but it is passed by reference instead of by value. |
+| test.cpp:22:19:22:21 | f5a | Parameter 'f5a' is the trivially copyable type $@ but it is passed by reference instead of by value. | file://:0:0:0:0 | const S1 | const S1 |

cpp/autosar/test/rules/A8-4-7/InParametersForNotCheapToCopyTypesNotPassedByReference.expected

@@ -1,2 +1,3 @@
-| test.cpp:23:12:23:14 | f8a | Parameter f8a is the trivially non-copyable type $@ but it is passed by value instead of by reference. | test.cpp:14:8:14:9 | S2 | S2 |
-| test.cpp:27:13:27:16 | f12a | Parameter f12a is the trivially non-copyable type $@ but it is passed by value instead of by reference. | test.cpp:14:8:14:9 | S2 | S2 |
+| test.cpp:25:12:25:14 | f8a | Parameter 'f8a' is the trivially non-copyable type $@ but it is passed by value instead of by reference. | test.cpp:16:8:16:9 | S2 | S2 |
+| test.cpp:29:13:29:16 | f12a | Parameter 'f12a' is the trivially non-copyable type $@ but it is passed by value instead of by reference. | test.cpp:16:8:16:9 | S2 | S2 |
+| test.cpp:70:13:70:16 | f17a | Parameter 'f17a' is the trivially non-copyable type $@ but it is passed by value instead of by reference. | test.cpp:60:8:60:9 | S4 | S4 |

cpp/autosar/test/rules/A8-4-7/test.cpp

@@ -1,5 +1,7 @@
+#include <array>
 #include <cstdint>
 #include <string>
+#include <vector>
 
 void f1(std::uint8_t f1a) {}  // COMPLIANT
 void f2(std::uint16_t f2a) {} // COMPLIANT
@@ -45,4 +47,25 @@ class C1 {};
 class C2 : public C1 {
 public:
   C2 &operator=(const C2 &); // COMPLIANT
-};
+};
+
+void f13(double f13a) {}       // COMPLIANT
+void f14(const double f14a) {} // COMPLIANT
+
+struct S3 {
+  int x;
+  S3() : x(0) {} // COMPLIANT
+};
+
+struct S4 {
+  ~S4() {} // non-trivial destructor
+};
+
+struct S5 {
+  const int y;
+  S5(int value) : y(value) {}
+};
+
+void f15(S3 f15a) {} // COMPLIANT
+void f17(S4 f17a) {} // NON_COMPLIANT (S4 has a non-trivial destructor)
+void f18(S5 f18a) {} // COMPLIANT

cpp/common/src/codingstandards/cpp/TrivialType.qll

@@ -281,6 +281,17 @@ predicate isTrivialType(Type t) {
   isTrivialType(t.getUnspecifiedType())
 }
 
+/** Holds if `t` is a trivially copyable type. */
+predicate isTriviallyCopyableType(Type t) {
+  isScalarType(t)
+  or
+  t instanceof TriviallyCopyableClass
+  or
+  isTriviallyCopyableType(t.(ArrayType).getBaseType())
+  or
+  isTriviallyCopyableType(t.getUnspecifiedType())
+}
+
 /** A POD type as defined by [basic.types]/9. */
 class PODType extends Type {
   PODType() {