From 0262926b1193db04d2ec0ede6fd31f3abd5d8c6a Mon Sep 17 00:00:00 2001 From: Matthias Koefferlein Date: Wed, 26 Oct 2022 10:40:20 +0200 Subject: [PATCH] Moved __deepcopy__ implementation and NotImplemented fallback from db.__init__ to C++ implementation for better performance and generalization. Added tests --- src/pya/pya/pyaCallables.cc | 32 +++++++- src/pya/pya/pyaCallables.h | 1 + src/pya/pya/pyaInternal.cc | 76 +++++++++++++++++-- src/pya/pya/pyaInternal.h | 16 +++- src/pya/pya/pyaModule.cc | 5 +- src/pya/pya/pyaUtils.h | 2 + .../distutils_src/klayout/db/__init__.py | 59 -------------- testdata/pymod/import_db.py | 13 ---- testdata/python/basic.py | 31 ++++++++ 9 files changed, 154 insertions(+), 81 deletions(-) diff --git a/src/pya/pya/pyaCallables.cc b/src/pya/pya/pyaCallables.cc index 1ac8f47fd..c0a782904 100644 --- a/src/pya/pya/pyaCallables.cc +++ b/src/pya/pya/pyaCallables.cc @@ -321,10 +321,20 @@ match_method (int mid, PyObject *self, PyObject *args, bool strict) } + } else if (meth && mt->fallback_not_implemented (mid)) { + + // one candidate, but needs checking whether compatibility is given - this avoid having to route NotImplemented over TypeError exceptions later + int i = 0; + for (gsi::MethodBase::argument_iterator a = meth->begin_arguments (); i < argc && a != meth->end_arguments (); ++a, ++i) { + if (! test_arg (*a, PyTuple_GetItem (args, i), true /*loose*/)) { + return 0; + } + } + } if (! meth) { - if (! strict) { + if (! strict || mt->fallback_not_implemented (mid)) { return 0; } else { throw tl::TypeError (tl::to_string (tr ("No overload with matching arguments"))); @@ -332,7 +342,7 @@ match_method (int mid, PyObject *self, PyObject *args, bool strict) } if (candidates > 1) { - if (! strict) { + if (! strict || mt->fallback_not_implemented (mid)) { return 0; } else { throw tl::TypeError (tl::to_string (tr ("Ambiguous overload variants - multiple method declarations match arguments"))); @@ -398,6 +408,19 @@ object_assign (PyObject *self, PyObject *args) return self; } +/** + * @brief Default implementation of "__deepcopy__" + */ +PyObject * +object_default_deepcopy_impl (PyObject *self, PyObject * /*args*/) +{ + PyObject *copy_method = PyObject_GetAttrString (self, "__copy__"); + tl_assert (copy_method != NULL); + + PythonRef empty_args (PyTuple_New (0)); + return PyObject_Call (copy_method, empty_args.get (), NULL); +} + /** * @brief Default implementation of "__ne__" */ @@ -638,6 +661,11 @@ method_adaptor (int mid, PyObject *self, PyObject *args) const gsi::MethodBase *meth = match_method (mid, self, args, true); + // method is not implemented + if (! meth) { + Py_RETURN_NOTIMPLEMENTED; + } + // handle special methods if (meth->smt () != gsi::MethodBase::None) { diff --git a/src/pya/pya/pyaCallables.h b/src/pya/pya/pyaCallables.h index b5dd11983..f6188b4fe 100644 --- a/src/pya/pya/pyaCallables.h +++ b/src/pya/pya/pyaCallables.h @@ -37,6 +37,7 @@ PyObject *object_default_ne_impl (PyObject *self, PyObject *args); PyObject *object_default_ge_impl (PyObject *self, PyObject *args); PyObject *object_default_le_impl (PyObject *self, PyObject *args); PyObject *object_default_gt_impl (PyObject *self, PyObject *args); +PyObject *object_default_deepcopy_impl (PyObject *self, PyObject *args); typedef PyObject *(*py_func_ptr_t) (PyObject *, PyObject *); diff --git a/src/pya/pya/pyaInternal.cc b/src/pya/pya/pyaInternal.cc index 35413acaa..86a56b443 100644 --- a/src/pya/pya/pyaInternal.cc +++ b/src/pya/pya/pyaInternal.cc @@ -37,7 +37,7 @@ namespace pya // MethodTableEntry implementation MethodTableEntry::MethodTableEntry (const std::string &name, bool st, bool prot) - : m_name (name), m_is_static (st), m_is_protected (prot), m_is_enabled (true), m_is_init (false) + : m_name (name), m_is_static (st), m_is_protected (prot), m_is_enabled (true), m_is_init (false), m_fallback_not_implemented (false) { } const std::string & @@ -64,6 +64,18 @@ MethodTableEntry::is_enabled () const return m_is_enabled; } +void +MethodTableEntry::set_fallback_not_implemented (bool f) +{ + m_fallback_not_implemented = f; +} + +bool +MethodTableEntry::fallback_not_implemented () const +{ + return m_fallback_not_implemented; +} + void MethodTableEntry::set_init (bool f) { @@ -424,6 +436,40 @@ static std::string extract_python_name (const std::string &name) } } +/** + * @brief Returns true, if the method with the given name shall fallback to NotImplemented + */ +static bool is_method_with_fallback (const std::string &name) +{ + if (name == "+") { + return true; + } else if (name == "-") { + return true; + } else if (name == "/") { + #if PY_MAJOR_VERSION < 3 + return false; + #else + return true; + #endif + } else if (name == "*") { + return true; + } else if (name == "%") { + return true; + } else if (name == "<<") { + return true; + } else if (name == ">>") { + return true; + } else if (name == "&") { + return true; + } else if (name == "|") { + return true; + } else if (name == "^") { + return true; + } else { + return false; + } +} + void MethodTable::add_method (const std::string &name, const gsi::MethodBase *mb) { @@ -511,11 +557,12 @@ MethodTable::add_method (const std::string &name, const gsi::MethodBase *mb) } else if (name == "dup" && mb->compatible_with_num_args (0)) { // If the object supports the dup method, then it is a good - // idea to define the __copy__ method. + // idea to define the __copy__ and __deepcopy__ method. add_method_basic ("__copy__", mb); + add_method_basic ("__deepcopy__", mb); add_method_basic (name, mb); - mp_module->add_python_doc (mb, tl::to_string (tr ("This method also implements '__copy__'"))); + mp_module->add_python_doc (mb, tl::to_string (tr ("This method also implements '__copy__' and '__deepcopy__'"))); } else { @@ -532,7 +579,8 @@ MethodTable::add_method (const std::string &name, const gsi::MethodBase *mb) } else { - add_method_basic (py_name, mb); + bool fb = is_method_with_fallback (name); + add_method_basic (py_name, mb, true, false, fb); if (name == "*") { // Supply a commutative multiplication version unless the operator is "*!" @@ -625,6 +673,18 @@ MethodTable::set_enabled (size_t mid, bool en) m_table [mid - m_method_offset].set_enabled (en); } +bool +MethodTable::fallback_not_implemented (size_t mid) const +{ + return m_table [mid - m_method_offset].fallback_not_implemented (); +} + +void +MethodTable::set_fallback_not_implemented (size_t mid, bool f) +{ + m_table [mid - m_method_offset].set_fallback_not_implemented (f); +} + bool MethodTable::is_init(size_t mid) const { @@ -743,7 +803,7 @@ MethodTable::finish () } void -MethodTable::add_method_basic (const std::string &name, const gsi::MethodBase *mb, bool enabled, bool init) +MethodTable::add_method_basic (const std::string &name, const gsi::MethodBase *mb, bool enabled, bool init, bool fallback_not_implemented) { bool st = mb->is_static () && ! init; @@ -758,6 +818,9 @@ MethodTable::add_method_basic (const std::string &name, const gsi::MethodBase *m if (init) { m_table.back ().set_init (true); } + if (fallback_not_implemented) { + m_table.back ().set_fallback_not_implemented (true); + } m_table.back ().add (mb); } else { @@ -773,6 +836,9 @@ MethodTable::add_method_basic (const std::string &name, const gsi::MethodBase *m if (init) { tl_assert (m_table [n->second].is_init ()); } + if (fallback_not_implemented) { + m_table.back ().set_fallback_not_implemented (true); + } } } diff --git a/src/pya/pya/pyaInternal.h b/src/pya/pya/pyaInternal.h index 0cbd071ae..4fc5ae3ee 100644 --- a/src/pya/pya/pyaInternal.h +++ b/src/pya/pya/pyaInternal.h @@ -67,6 +67,9 @@ public: void set_enabled (bool en); bool is_enabled () const; + void set_fallback_not_implemented (bool en); + bool fallback_not_implemented () const; + void set_init(bool f); bool is_init () const; @@ -91,6 +94,7 @@ private: bool m_is_protected : 1; bool m_is_enabled : 1; bool m_is_init : 1; + bool m_fallback_not_implemented : 1; std::vector m_methods; }; @@ -169,6 +173,16 @@ public: */ void set_enabled (size_t mid, bool en); + /** + * @brief Returns true if the method has a NotImplemented fallback + */ + bool fallback_not_implemented (size_t mid) const; + + /** + * @brief Sets a value indicating that the method has a fallback to NotImplemented for non-matching arguments + */ + void set_fallback_not_implemented (size_t mid, bool f); + /** * @brief Returns true if the method is an initializer */ @@ -277,7 +291,7 @@ private: std::vector > m_property_table; PythonModule *mp_module; - void add_method_basic (const std::string &name, const gsi::MethodBase *mb, bool enabled = true, bool init = false); + void add_method_basic (const std::string &name, const gsi::MethodBase *mb, bool enabled = true, bool init = false, bool fallback_not_implemented = false); void add_setter_basic (const std::string &name, const gsi::MethodBase *setter); void add_getter_basic (const std::string &name, const gsi::MethodBase *getter); bool is_property_setter (bool st, const std::string &name); diff --git a/src/pya/pya/pyaModule.cc b/src/pya/pya/pyaModule.cc index 39a8a1a09..fa9c1b48c 100644 --- a/src/pya/pya/pyaModule.cc +++ b/src/pya/pya/pyaModule.cc @@ -484,7 +484,10 @@ public: PyMethodDef *method = mp_module->make_method_def (); method->ml_name = mp_module->make_string (name); - if (mt->is_init (mid)) { + if (name == "__deepcopy__") { + // Special handling needed as the memo argument needs to be ignored + method->ml_meth = &object_default_deepcopy_impl; + } else if (mt->is_init (mid)) { method->ml_meth = (PyCFunction) get_method_init_adaptor (mid); } else { method->ml_meth = (PyCFunction) get_method_adaptor (mid); diff --git a/src/pya/pya/pyaUtils.h b/src/pya/pya/pyaUtils.h index 6f1294f72..05f684c86 100644 --- a/src/pya/pya/pyaUtils.h +++ b/src/pya/pya/pyaUtils.h @@ -29,6 +29,8 @@ namespace pya { + + /** * Some helper macros that translate C++ exceptions into Python errors */ diff --git a/src/pymod/distutils_src/klayout/db/__init__.py b/src/pymod/distutils_src/klayout/db/__init__.py index 362acbffd..fee5eb8c9 100644 --- a/src/pymod/distutils_src/klayout/db/__init__.py +++ b/src/pymod/distutils_src/klayout/db/__init__.py @@ -7,65 +7,6 @@ from klayout.db.pcell_declaration_helper import PCellDeclarationHelper __all__ = klayout.dbcore.__all__ + ["PCellDeclarationHelper"] # type: ignore -# Implementing deepcopy of common objects -# Point-like classes -PointLike = (Point, DPoint, DVector, Vector) - - -def pyaPoint__deepcopy__(self, memo): - return self.dup() - - -def convert_type_error_to_not_implemented(cls, method): - """If cls.method exists raises a TypeError, patch it so - it returns a NotImplemented error instead. - - - """ - if not hasattr(cls, method): - return - - old_func = getattr(cls, method) - - @functools.wraps(old_func) - def new_func(*args, **kwargs): - try: - return old_func(*args, **kwargs) - except TypeError: - return NotImplemented - try: - setattr(cls, method, new_func) - except TypeError: - # Some classes are immutable and cannot be changed. - # At the time of writing, this happens to (_StaticAttribute, _AmbiguousMethodDispatcher, _Iterator, _Signal).__or__ - return - -for PClass in PointLike: - PClass.__deepcopy__ = pyaPoint__deepcopy__ # type: ignore - -for cls in klayout.dbcore.__dict__.values(): - if not isinstance(cls, type): # skip if not a class - continue - for method in ( - "__add__", - "__sub__", - "__mul__", - "__matmul__", - "__truediv__", - "__floordiv__", - "__mod__", - "__divmod__", - "__pow__", - "__lshift__", - "__rshift__", - "__and__", - "__xor__", - "__or__", - ): - # list of methods extracted from https://docs.python.org/3.7/reference/datamodel.html#emulating-numeric-types - convert_type_error_to_not_implemented(cls, method) - - # If class has from_s, to_s, and assign, use them to # enable serialization. for name, cls in klayout.dbcore.__dict__.items(): diff --git a/testdata/pymod/import_db.py b/testdata/pymod/import_db.py index aa88e8221..d16f131aa 100755 --- a/testdata/pymod/import_db.py +++ b/testdata/pymod/import_db.py @@ -41,19 +41,6 @@ class BasicTest(unittest.TestCase): v.read(os.path.join(os.path.dirname(__file__), "..", "gds", "t10.gds")) self.assertEqual(v.top_cell().name, "RINGO") - def test_4(self): - class RMulObject: - def __init__(self, factor): - self.factor = factor - def __rmul__(self, point): - return point * self.factor - def __radd__(self, point): - return point + db.Vector(1,1) * self.factor - - p = db.Point(1, 0) - fac2 = RMulObject(2) - self.assertEqual(p * 2, p * fac2) # p.__mul__(fac2) should return NotImplemented, which will call fac2.__rmul__(p) - self.assertEqual(db.Point(3,2), p + fac2) # run unit tests if __name__ == '__main__': suite = unittest.TestSuite() diff --git a/testdata/python/basic.py b/testdata/python/basic.py index 4c524342f..7595babdc 100644 --- a/testdata/python/basic.py +++ b/testdata/python/basic.py @@ -21,6 +21,7 @@ import unittest import os import sys import gc +import copy # Set this to True to disable some tests involving exceptions leak_check = "TEST_LEAK_CHECK" in os.environ @@ -3073,6 +3074,36 @@ class BasicTest(unittest.TestCase): go = None self.assertEqual(pya.GObject.g_inst_count(), gc) + # fallback to __rmul__ for not implemented __mul__ + + def test_90(self): + class RMulObject: + def __init__(self, factor): + self.factor = factor + def __rmul__(self, point): + return point * self.factor + def __radd__(self, point): + return point + pya.Vector(1,1) * self.factor + + p = pya.Point(1, 0) + fac2 = RMulObject(2) + self.assertEqual(p * 2, p * fac2) # p.__mul__(fac2) should return NotImplemented, which will call fac2.__rmul__(p) + self.assertEqual(pya.Point(3,2), p + fac2) + + # copy and deepcopy + + def test_91(self): + + p = pya.Point(1, 0) + pc = copy.copy(p) + pdc = copy.deepcopy(p) + + pdc.x = 4 + pc.x = 3 + p.x = 2 + self.assertEqual(p.x, 2) + self.assertEqual(pc.x, 3) + self.assertEqual(pdc.x, 4) # run unit tests if __name__ == '__main__':