Introduce ::clif::python::IsWrapperTypeInstance(), excluding mocks.

**Please request a rollback only as a last resort**. This CL is expected to only affect test code (hopefully nobody uses mocks in production). To buy time for fixing up a broken test, please use `skipTest` as in e.g. cl/550935761.

See b/289319214 for background.

Also fix a minor long-standing bug in generated C API code: Avoid clobbering unexpected `PyObject_IsInstance()` errors.

PiperOrigin-RevId: 550940223

clif/python/class_test.py

@@ -226,12 +226,12 @@ def testStruct(self):
           }
           Py_DECREF(base);
         }
-        if (PyObject_IsInstance(py, reinterpret_cast<PyObject*>(wrapper_Type))) {
+        if (::clif::python::IsWrapperTypeInstance(py, wrapper_Type)) {
           if (!base) {
             return ::clif::python::Get(reinterpret_cast<wrapper*>(py)->cpp);
           }
           PyErr_Format(PyExc_ValueError, "can't convert %%s %%s to StructTy*", ClassName(py), ClassType(py));
-        } else {
+        } else if (!PyErr_Occurred()) {
           PyErr_Format(PyExc_TypeError, "expecting %%s instance, got %%s %%s", wrapper_Type->tp_name, ClassName(py), ClassType(py));
         }
         return nullptr;
@@ -431,12 +431,12 @@ def testCppDerivedStruct(self):
           }
           Py_DECREF(base);
         }
-        if (PyObject_IsInstance(py, reinterpret_cast<PyObject*>(wrapper_Type))) {
+        if (::clif::python::IsWrapperTypeInstance(py, wrapper_Type)) {
           if (!base) {
             return ::clif::python::Get(reinterpret_cast<wrapper*>(py)->cpp);
           }
           PyErr_Format(PyExc_ValueError, "can't convert %%s %%s to StructTy*", ClassName(py), ClassType(py));
-        } else {
+        } else if (!PyErr_Occurred()) {
           PyErr_Format(PyExc_TypeError, "expecting %%s instance, got %%s %%s", wrapper_Type->tp_name, ClassName(py), ClassType(py));
         }
         return nullptr;
@@ -627,12 +627,12 @@ def testPythonImportedDerivedStruct(self):
           }
           Py_DECREF(base);
         }
-        if (PyObject_IsInstance(py, reinterpret_cast<PyObject*>(wrapper_Type))) {
+        if (::clif::python::IsWrapperTypeInstance(py, wrapper_Type)) {
           if (!base) {
             return ::clif::python::Get(reinterpret_cast<wrapper*>(py)->cpp);
           }
           PyErr_Format(PyExc_ValueError, "can't convert %%s %%s to StructTy*", ClassName(py), ClassType(py));
-        } else {
+        } else if (!PyErr_Occurred()) {
           PyErr_Format(PyExc_TypeError, "expecting %%s instance, got %%s %%s", wrapper_Type->tp_name, ClassName(py), ClassType(py));
         }
         return nullptr;
@@ -899,12 +899,12 @@ def testNestedStruct(self):
           }
           Py_DECREF(base);
         }
-        if (PyObject_IsInstance(py, reinterpret_cast<PyObject*>(wrapper_Type))) {
+        if (::clif::python::IsWrapperTypeInstance(py, wrapper_Type)) {
           if (!base) {
             return ::clif::python::Get(reinterpret_cast<wrapper*>(py)->cpp);
           }
           PyErr_Format(PyExc_ValueError, "can't convert %%s %%s to OutKlass::InnKlass*", ClassName(py), ClassType(py));
-        } else {
+        } else if (!PyErr_Occurred()) {
           PyErr_Format(PyExc_TypeError, "expecting %%s instance, got %%s %%s", wrapper_Type->tp_name, ClassName(py), ClassType(py));
         }
         return nullptr;
@@ -1002,12 +1002,12 @@ def testNestedStruct(self):
           }
           Py_DECREF(base);
         }
-        if (PyObject_IsInstance(py, reinterpret_cast<PyObject*>(wrapper_Type))) {
+        if (::clif::python::IsWrapperTypeInstance(py, wrapper_Type)) {
           if (!base) {
             return ::clif::python::Get(reinterpret_cast<wrapper*>(py)->cpp);
           }
           PyErr_Format(PyExc_ValueError, "can't convert %%s %%s to OutKlass*", ClassName(py), ClassType(py));
-        } else {
+        } else if (!PyErr_Occurred()) {
           PyErr_Format(PyExc_TypeError, "expecting %%s instance, got %%s %%s", wrapper_Type->tp_name, ClassName(py), ClassType(py));
         }
         return nullptr;
@@ -1234,12 +1234,12 @@ def testStructProperty(self):
           }
           Py_DECREF(base);
         }
-        if (PyObject_IsInstance(py, reinterpret_cast<PyObject*>(wrapper_Type))) {
+        if (::clif::python::IsWrapperTypeInstance(py, wrapper_Type)) {
           if (!base) {
             return ::clif::python::Get(reinterpret_cast<wrapper*>(py)->cpp);
           }
           PyErr_Format(PyExc_ValueError, "can't convert %%s %%s to StructTy*", ClassName(py), ClassType(py));
-        } else {
+        } else if (!PyErr_Occurred()) {
           PyErr_Format(PyExc_TypeError, "expecting %%s instance, got %%s %%s", wrapper_Type->tp_name, ClassName(py), ClassType(py));
         }
         return nullptr;
@@ -1428,12 +1428,12 @@ def testStructUnProperty(self):
           }
           Py_DECREF(base);
         }
-        if (PyObject_IsInstance(py, reinterpret_cast<PyObject*>(wrapper_Type))) {
+        if (::clif::python::IsWrapperTypeInstance(py, wrapper_Type)) {
           if (!base) {
             return ::clif::python::Get(reinterpret_cast<wrapper*>(py)->cpp);
           }
           PyErr_Format(PyExc_ValueError, "can't convert %%s %%s to StructTy*", ClassName(py), ClassType(py));
-        } else {
+        } else if (!PyErr_Occurred()) {
           PyErr_Format(PyExc_TypeError, "expecting %%s instance, got %%s %%s", wrapper_Type->tp_name, ClassName(py), ClassType(py));
         }
         return nullptr;
@@ -2025,12 +2025,12 @@ def testVirtualStruct(self):
           }
           Py_DECREF(base);
         }
-        if (PyObject_IsInstance(py, reinterpret_cast<PyObject*>(wrapper_Type))) {
+        if (::clif::python::IsWrapperTypeInstance(py, wrapper_Type)) {
           if (!base) {
             return ::clif::python::Get(reinterpret_cast<wrapper*>(py)->cpp);
           }
           PyErr_Format(PyExc_ValueError, "can't convert %%s %%s to StructCpp*", ClassName(py), ClassType(py));
-        } else {
+        } else if (!PyErr_Occurred()) {
           PyErr_Format(PyExc_TypeError, "expecting %%s instance, got %%s %%s", wrapper_Type->tp_name, ClassName(py), ClassType(py));
         }
         return nullptr;
@@ -2273,12 +2273,12 @@ def testIteratorStruct(self):
           }
           Py_DECREF(base);
         }
-        if (PyObject_IsInstance(py, reinterpret_cast<PyObject*>(wrapper_Type))) {
+        if (::clif::python::IsWrapperTypeInstance(py, wrapper_Type)) {
           if (!base) {
             return ::clif::python::Get(reinterpret_cast<wrapper*>(py)->cpp);
           }
           PyErr_Format(PyExc_ValueError, "can't convert %%s %%s to StructCpp*", ClassName(py), ClassType(py));
-        } else {
+        } else if (!PyErr_Occurred()) {
           PyErr_Format(PyExc_TypeError, "expecting %%s instance, got %%s %%s", wrapper_Type->tp_name, ClassName(py), ClassType(py));
         }
         return nullptr;

clif/python/clif_types.py

@@ -499,13 +499,13 @@ def GenThisPointerFunc(cname, w='wrapper', final=False):
     _I=''  # pylint: disable=bad-whitespace,invalid-name
   else:
     # as_Base returned an error / wrong capsule
-    yield I+'if (PyObject_IsInstance(py, %s)) {' % AsPyObj(t)
+    yield I+'if (::clif::python::IsWrapperTypeInstance(py, %s)) {' % t
     yield I+I+'if (!base) {'  # base set in _GenBaseCapsule
     yield I+I+I+return_this_cpp
     yield I+I+'}'
     yield I+I+('PyErr_Format(PyExc_ValueError, "can\'t convert %s %s to {}*", '
                'ClassName(py), ClassType(py));').format(cname)
-    yield I+'} else {'
+    yield I+'} else if (!PyErr_Occurred()) {'
     _I=I  # pylint: disable=bad-whitespace,invalid-name
   yield _I+I+('PyErr_Format(PyExc_TypeError, "expecting %s instance, got %s %s"'
               ', {}->tp_name, ClassName(py), ClassType(py));'.format(t))

clif/python/runtime.cc

@@ -14,6 +14,7 @@
 
 #include "clif/python/runtime.h"
 
+#include <cassert>
 #include <initializer_list>
 #include <system_error>  // NOLINT(build/c++11)
 
@@ -228,6 +229,42 @@ PyObject* ArgError(const char func[],
 
 namespace python {
 
+namespace {
+
+int FilterMockUsingPragmaticHeuristics(PyObject* inst) {
+  // Definition of the class variable leveraged as "is a mock" indicator:
+  // https://github.com/python/cpython/blob/7d41ead9191a18bc473534f6f8bd1095f2232cc2/Lib/unittest/mock.py#L426
+  // The runtime overhead is very small: a single attribute lookup.
+  // Likelihood of false negatives: practically zero.
+  // Likelihood of false positives: extremely unlikely.
+  // It actually seems much more likely that this function works as intended
+  // for hypothetical non-unittest mocks, rather than producing a false
+  // positive for an object that is not a mock.
+  assert(PyType_Check(inst) == 0);
+  PyTypeObject* typ = reinterpret_cast<PyTypeObject*>(Py_TYPE(inst));
+  PyObject* mock_attr = PyObject_GetAttrString(reinterpret_cast<PyObject*>(typ),
+                                               "_mock_return_value");
+  if (mock_attr != nullptr) {
+    Py_DECREF(mock_attr);
+    return 0;  // "Yes this is a mock."
+  }
+  if (PyErr_ExceptionMatches(PyExc_AttributeError)) {
+    PyErr_Clear();
+    return 1;  // "No this is not a mock."
+  }
+  return -1;  // Unexpected error.
+}
+
+}  // namespace
+
+int IsWrapperTypeInstance(PyObject* inst, PyTypeObject* cls) {
+  int stat = PyObject_IsInstance(inst, reinterpret_cast<PyObject*>(cls));
+  if (stat == 1) {
+    return FilterMockUsingPragmaticHeuristics(inst);
+  }
+  return stat;
+}
+
 std::string ExcStr(bool add_type) {
   PyObject* exc, *val, *tb;
   PyErr_Fetch(&exc, &val, &tb);

clif/python/runtime.h

@@ -32,6 +32,13 @@ extern "C" int Clif_PyType_Inconstructible(PyObject*, PyObject*, PyObject*);
 namespace clif {
 namespace python {
 
+// Similar to https://docs.python.org/3/c-api/object.html#c.PyObject_IsInstance
+// but a simple heuristic is used to filter out `unittest.mock` objects
+// (see implementation for details).
+// Return `1` if `inst` is an instance of the class `cls` or a subclass of
+// `cls`, or `0` if not. On error, returns `-1` and sets an exception.
+int IsWrapperTypeInstance(PyObject* inst, PyTypeObject* cls);
+
 std::string ExcStr(bool add_type = true);
 void ThrowExcStrIfCppExceptionsEnabled();
 void LogCallbackPythonError(PyObject* callable, const char* return_typeid_name);
@@ -49,6 +56,7 @@ T* Get(const clif::Instance<T>& cpp, bool set_err = true) {
   }
   return d;
 }
+
 }  // namespace python
 
 // Returns py.__class__.__name__ (needed for PY2 old style classes).

clif/testing/classes.h

@@ -38,6 +38,8 @@ class K {
   int i_;
 };
 
+inline int PassK(const K& k_inst) { return k_inst.get() * 10; }
+
 class NoDefaultConstructor {
  private:
   explicit NoDefaultConstructor() {}

clif/testing/python/classes.clif

@@ -23,6 +23,8 @@ from "clif/testing/classes.h":
       i: int = property(`get`, `set`)
       i2: int = property(`get2`)
 
+    def `PassK` as PassKlass(k_inst: Klass) -> int
+
     class Derived(Klass):
       """This class also has a "docstring".
 

clif/testing/python/classes_test.py

@@ -14,6 +14,8 @@
 
 """Tests for clif.testing.python.classes."""
 
+from unittest import mock
+
 from absl.testing import absltest
 from absl.testing import parameterized
 
@@ -34,6 +36,22 @@ def testKlass(self):
     with self.assertRaises((AttributeError, TypeError)):
       k.i2 = 0
 
+  def testMockIsRejected(self):
+    k_inst = classes.Klass(3)
+    self.assertEqual(classes.PassKlass(k_inst), 30)
+    d_inst = classes.Derived.Init(4, 0)
+    self.assertEqual(classes.PassKlass(d_inst), 40)
+    with mock.patch.object(classes, 'Klass', autospec=True):
+      k_mock = classes.Klass(3)
+      with self.assertRaises(TypeError) as ctx:
+        classes.PassKlass(k_mock)
+      self.assertIn('Mock', str(ctx.exception))
+    with mock.patch.object(classes, 'Derived', autospec=True):
+      d_mock = classes.Derived.Init(4, 0)
+      with self.assertRaises(TypeError) as ctx:
+        classes.PassKlass(d_mock)
+      self.assertIn('Mock', str(ctx.exception))
+
   def testDerivedClassDocstring(self):
     # Nothing special about this being a derived class; that is just the
     # one our test .clif file has a docstring on.