Merge pull request #4213 from rdmarsh2/rdmarsh2/cpp/explicit-conversion-perf

C++: Improve performance of getExplicitlyConverted

Author: geoffw0

cpp/ql/src/semmle/code/cpp/exprs/Expr.qll

@@ -402,7 +402,7 @@ class Expr extends StmtParent, @expr {
    */
   predicate hasImplicitConversion() {
     exists(Expr e |
-      exprconv(underlyingElement(this), unresolveElement(e)) and e.(Cast).isImplicit()
+      exprconv(underlyingElement(this), unresolveElement(e)) and e.(Conversion).isImplicit()
     )
   }
 
@@ -414,7 +414,7 @@ class Expr extends StmtParent, @expr {
    */
   predicate hasExplicitConversion() {
     exists(Expr e |
-      exprconv(underlyingElement(this), unresolveElement(e)) and not e.(Cast).isImplicit()
+      exprconv(underlyingElement(this), unresolveElement(e)) and not e.(Conversion).isImplicit()
     )
   }
 
@@ -453,12 +453,14 @@ class Expr extends StmtParent, @expr {
    * cast from B to C. Only (1) and (2) would be included.
    */
   Expr getExplicitlyConverted() {
-    // result is this or one of its conversions
-    result = this.getConversion*() and
-    // result is not an implicit conversion - it's either the expr or an explicit cast
-    (result = this or not result.(Cast).isImplicit()) and
-    // there is no further explicit conversion after result
-    not exists(Cast other | other = result.getConversion+() and not other.isImplicit())
+    // For performance, we avoid a full transitive closure over `getConversion`.
+    // Since there can be several implicit conversions before and after an
+    // explicit conversion, use `getImplicitlyConverted` to step over them
+    // cheaply. Then, if there is an explicit conversion following the implict
+    // conversion sequence, recurse to handle multiple explicit conversions.
+    if this.getImplicitlyConverted().hasExplicitConversion()
+    then result = this.getImplicitlyConverted().getConversion().getExplicitlyConverted()
+    else result = this
   }
 
   /**

cpp/ql/test/query-tests/Likely Bugs/Underspecified Functions/MistypedFunctionArguments.expected

@@ -1,6 +1,4 @@
-| test.c:33:3:33:19 | call to not_yet_declared2 | Calling $@: argument $@ of type $@ is incompatible with parameter $@. | test.c:32:3:32:3 | not_yet_declared2 | not_yet_declared2 | test.c:33:21:33:22 | ca | ca | file://:0:0:0:0 | int * | int * | test.c:76:24:76:26 | p#0 | int p#0 |
 | test.c:33:3:33:19 | call to not_yet_declared2 | Calling $@: argument $@ of type $@ is incompatible with parameter $@. | test.c:32:3:32:3 | not_yet_declared2 | not_yet_declared2 | test.c:33:21:33:22 | ca | ca | file://:0:0:0:0 | int[4] | int[4] | test.c:76:24:76:26 | p#0 | int p#0 |
-| test.c:33:3:33:19 | call to not_yet_declared2 | Calling $@: argument $@ of type $@ is incompatible with parameter $@. | test.c:76:6:76:22 | not_yet_declared2 | not_yet_declared2 | test.c:33:21:33:22 | ca | ca | file://:0:0:0:0 | int * | int * | test.c:76:24:76:26 | p#0 | int p#0 |
 | test.c:33:3:33:19 | call to not_yet_declared2 | Calling $@: argument $@ of type $@ is incompatible with parameter $@. | test.c:76:6:76:22 | not_yet_declared2 | not_yet_declared2 | test.c:33:21:33:22 | ca | ca | file://:0:0:0:0 | int[4] | int[4] | test.c:76:24:76:26 | p#0 | int p#0 |
 | test.c:40:3:40:29 | call to declared_empty_defined_with | Calling $@: argument $@ of type $@ is incompatible with parameter $@. | test.c:77:6:77:32 | declared_empty_defined_with | declared_empty_defined_with | test.c:40:31:40:32 | & ... | & ... | file://:0:0:0:0 | int * | int * | test.c:77:38:77:38 | x | int x |
 | test.c:44:3:44:27 | call to not_declared_defined_with | Calling $@: argument $@ of type $@ is incompatible with parameter $@. | test.c:80:6:80:30 | not_declared_defined_with | not_declared_defined_with | test.c:44:29:44:31 | 4 | 4 | file://:0:0:0:0 | long long | long long | test.c:80:36:80:36 | x | int x |