Merge pull request github#3768 from geoffw0/copymove

C++: Clean up ConversionConstructor.

Author: jbj

cpp/ql/src/semmle/code/cpp/MemberFunction.qll

@@ -214,6 +214,9 @@ abstract class ImplicitConversionFunction extends MemberFunction {
 }
 
 /**
+ * DEPRECATED: as of C++11 this class does not correspond perfectly with the
+ * language definition of a converting constructor.
+ *
  * A C++ constructor that also defines an implicit conversion. For example the
  * function `MyClass` in the following code is a `ConversionConstructor`:
  * ```
@@ -225,15 +228,16 @@ abstract class ImplicitConversionFunction extends MemberFunction {
  * };
  * ```
  */
-class ConversionConstructor extends Constructor, ImplicitConversionFunction {
+deprecated class ConversionConstructor extends Constructor, ImplicitConversionFunction {
   ConversionConstructor() {
     strictcount(Parameter p | p = getAParameter() and not p.hasInitializer()) = 1 and
-    not hasSpecifier("explicit") and
-    not this instanceof CopyConstructor
+    not hasSpecifier("explicit")
   }
 
   override string getAPrimaryQlClass() {
-    not this instanceof MoveConstructor and result = "ConversionConstructor"
+    not this instanceof CopyConstructor and
+    not this instanceof MoveConstructor and
+    result = "ConversionConstructor"
   }
 
   /** Gets the type this `ConversionConstructor` takes as input. */

cpp/ql/src/semmle/code/cpp/models/implementations/MemberFunction.qll

@@ -7,9 +7,17 @@ import semmle.code.cpp.models.interfaces.DataFlow
 import semmle.code.cpp.models.interfaces.Taint
 
 /**
- * Model for C++ conversion constructors.
+ * Model for C++ conversion constructors. As of C++11 this does not correspond
+ * perfectly with the language definition of a converting constructor, however,
+ * it does correspond with the constructors we are confident taint should flow
+ * through.
  */
-class ConversionConstructorModel extends ConversionConstructor, TaintFunction {
+class ConversionConstructorModel extends Constructor, TaintFunction {
+  ConversionConstructorModel() {
+    strictcount(Parameter p | p = getAParameter() and not p.hasInitializer()) = 1 and
+    not hasSpecifier("explicit")
+  }
+
   override predicate hasTaintFlow(FunctionInput input, FunctionOutput output) {
     // taint flow from the first constructor argument to the returned object
     input.isParameter(0) and

cpp/ql/test/library-tests/functions/functions/Functions2.expected

@@ -20,7 +20,7 @@
 | functions.cpp:23:7:23:11 | Table | Class | functions.cpp:30:8:30:13 | insert |  |
 | functions.cpp:33:7:33:13 | MyClass | Class | functions.cpp:33:7:33:7 | operator= |  |
 | functions.cpp:33:7:33:13 | MyClass | Class | functions.cpp:36:2:36:8 | MyClass | Constructor, NoArgConstructor, getAConstructor() |
-| functions.cpp:33:7:33:13 | MyClass | Class | functions.cpp:37:2:37:8 | MyClass | Constructor, ConversionConstructor, getAConstructor() |
+| functions.cpp:33:7:33:13 | MyClass | Class | functions.cpp:37:2:37:8 | MyClass | Constructor, getAConstructor() |
 | functions.cpp:33:7:33:13 | MyClass | Class | functions.cpp:38:2:38:8 | MyClass | Constructor, CopyConstructor, getAConstructor() |
-| functions.cpp:33:7:33:13 | MyClass | Class | functions.cpp:39:2:39:8 | MyClass | Constructor, ConversionConstructor, MoveConstructor, getAConstructor() |
+| functions.cpp:33:7:33:13 | MyClass | Class | functions.cpp:39:2:39:8 | MyClass | Constructor, MoveConstructor, getAConstructor() |
 | functions.cpp:33:7:33:13 | MyClass | Class | functions.cpp:40:2:40:13 | operator int | ConversionOperator |

cpp/ql/test/library-tests/functions/functions/Functions2.ql

@@ -19,9 +19,6 @@ string describe(Class c, MemberFunction f) {
   f instanceof Destructor and
   result = "Destructor"
   or
-  f instanceof ConversionConstructor and
-  result = "ConversionConstructor"
-  or
   f instanceof CopyConstructor and
   result = "CopyConstructor"
   or