From b182224463b153420ac4eb1a4ea6c9f86dae7ad5 Mon Sep 17 00:00:00 2001
From: rdb <git@rdb.name>
Date: Mon, 5 Dec 2016 17:22:24 -0500
Subject: [PATCH] interrogate: fix issues with abstract classes and covariance
 (fixes EggPolygon constructor)

---
 dtool/src/cppparser/cppFunctionType.cxx |  9 +++--
 dtool/src/cppparser/cppFunctionType.h   |  2 +-
 dtool/src/cppparser/cppStructType.cxx   | 54 ++++++++-----------------
 3 files changed, 24 insertions(+), 41 deletions(-)

diff --git a/dtool/src/cppparser/cppFunctionType.cxx b/dtool/src/cppparser/cppFunctionType.cxx
index 371b714c2d..1872ea11f5 100644
--- a/dtool/src/cppparser/cppFunctionType.cxx
+++ b/dtool/src/cppparser/cppFunctionType.cxx
@@ -326,14 +326,17 @@ as_function_type() {
  * This is similar to is_equal(), except it is more forgiving: it considers
  * the functions to be equivalent only if the return type and the types of all
  * parameters match.
+ *
+ * Note that this isn't symmetric to account for covariant return types.
  */
 bool CPPFunctionType::
-is_equivalent_function(const CPPFunctionType &other) const {
-  if (!_return_type->is_equivalent(*other._return_type)) {
+match_virtual_override(const CPPFunctionType &other) const {
+  if (!_return_type->is_equivalent(*other._return_type) &&
+      !_return_type->is_convertible_to(other._return_type)) {
     return false;
   }
 
-  if (_flags != other._flags) {
+  if (((_flags ^ other._flags) & ~(F_override | F_final)) != 0) {
     return false;
   }
 
diff --git a/dtool/src/cppparser/cppFunctionType.h b/dtool/src/cppparser/cppFunctionType.h
index f08de51e45..1e44681f13 100644
--- a/dtool/src/cppparser/cppFunctionType.h
+++ b/dtool/src/cppparser/cppFunctionType.h
@@ -84,7 +84,7 @@ public:
 
   virtual CPPFunctionType *as_function_type();
 
-  bool is_equivalent_function(const CPPFunctionType &other) const;
+  bool match_virtual_override(const CPPFunctionType &other) const;
 
   CPPIdentifier *_class_owner;
 
diff --git a/dtool/src/cppparser/cppStructType.cxx b/dtool/src/cppparser/cppStructType.cxx
index 73712c66ad..afb03dd463 100644
--- a/dtool/src/cppparser/cppStructType.cxx
+++ b/dtool/src/cppparser/cppStructType.cxx
@@ -378,6 +378,10 @@ is_constructible(const CPPType *given_type) const {
     }
   }
 
+  if (is_abstract()) {
+    return false;
+  }
+
   // Check for a different constructor.
   CPPFunctionGroup *fgroup = get_constructor();
   if (fgroup != (CPPFunctionGroup *)NULL) {
@@ -444,6 +448,10 @@ is_destructible() const {
  */
 bool CPPStructType::
 is_default_constructible(CPPVisibility min_vis) const {
+  if (is_abstract()) {
+    return false;
+  }
+
   CPPInstance *constructor = get_default_constructor();
   if (constructor != (CPPInstance *)NULL) {
     // It has a default constructor.
@@ -498,24 +506,6 @@ is_default_constructible(CPPVisibility min_vis) const {
     }
   }
 
-  // Check that we don't have pure virtual methods.
-  CPPScope::Functions::const_iterator fi;
-  for (fi = _scope->_functions.begin();
-       fi != _scope->_functions.end();
-       ++fi) {
-    CPPFunctionGroup *fgroup = (*fi).second;
-    CPPFunctionGroup::Instances::const_iterator ii;
-    for (ii = fgroup->_instances.begin();
-         ii != fgroup->_instances.end();
-         ++ii) {
-      CPPInstance *inst = (*ii);
-      if (inst->_storage_class & CPPInstance::SC_pure_virtual) {
-        // Here's a pure virtual function.
-        return false;
-      }
-    }
-  }
-
   return true;
 }
 
@@ -524,6 +514,10 @@ is_default_constructible(CPPVisibility min_vis) const {
  */
 bool CPPStructType::
 is_copy_constructible(CPPVisibility min_vis) const {
+  if (is_abstract()) {
+    return false;
+  }
+
   CPPInstance *constructor = get_copy_constructor();
   if (constructor != (CPPInstance *)NULL) {
     // It has a copy constructor.
@@ -581,24 +575,6 @@ is_copy_constructible(CPPVisibility min_vis) const {
     }
   }
 
-  // Check that we don't have pure virtual methods.
-  CPPScope::Functions::const_iterator fi;
-  for (fi = _scope->_functions.begin();
-       fi != _scope->_functions.end();
-       ++fi) {
-    CPPFunctionGroup *fgroup = (*fi).second;
-    CPPFunctionGroup::Instances::const_iterator ii;
-    for (ii = fgroup->_instances.begin();
-         ii != fgroup->_instances.end();
-         ++ii) {
-      CPPInstance *inst = (*ii);
-      if (inst->_storage_class & CPPInstance::SC_pure_virtual) {
-        // Here's a pure virtual function.
-        return false;
-      }
-    }
-  }
-
   return true;
 }
 
@@ -620,6 +596,10 @@ is_move_constructible(CPPVisibility min_vis) const {
       return false;
     }
 
+    if (is_abstract()) {
+      return false;
+    }
+
     return true;
   }
 
@@ -1214,7 +1194,7 @@ get_virtual_funcs(VFunctions &funcs) const {
           CPPFunctionType *new_ftype = new_inst->_type->as_function_type();
           assert(new_ftype != (CPPFunctionType *)NULL);
 
-          if (new_ftype->is_equivalent_function(*base_ftype)) {
+          if (new_ftype->match_virtual_override(*base_ftype)) {
             // It's a match!  We now know it's virtual.  Erase this function
             // from the list, so we can add it back in below.
             funcs.erase(vfi);