diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5a1b7195..b60a1c4f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -47,9 +47,8 @@ jobs: python: "3.9" - ubuntu: "20.04" python: "3.10" - # 1 failing test: pickle_compatibility_test.py - # - ubuntu: "20.04" - # python: "3.11" + - ubuntu: "20.04" + python: "3.11" name: "Ubuntu ${{ matrix.ubuntu }} - Python ${{ matrix.python }}" runs-on: ubuntu-latest diff --git a/clif/pybind11/cindex/extractor.py b/clif/pybind11/cindex/extractor.py index 0c6e9df1..0b9347e8 100644 --- a/clif/pybind11/cindex/extractor.py +++ b/clif/pybind11/cindex/extractor.py @@ -28,7 +28,7 @@ def _get_pointee_name(type_name: Text) -> Text: return type_name -class Type(object): +class Type: """Wraps a clang or a CLIF type.""" def __init__(self, name: Text, is_pointer: bool, is_reference: bool, @@ -115,7 +115,7 @@ def pointee_name(self): return '' -class Function(object): +class Function: """Wraps a clang or a CLIF Function.""" def __init__(self, fq_name: Text, is_pure_virtual: bool, @@ -191,7 +191,7 @@ def is_overloaded(self, is_overloaded: bool) -> None: self._is_overloaded = is_overloaded -class Module(object): +class Module: """Wraps a clang AST.""" def __init__(self, tu: TranslationUnit): diff --git a/clif/pybind11/function_lib.py b/clif/pybind11/function_lib.py index 06c09ba3..b0534774 100644 --- a/clif/pybind11/function_lib.py +++ b/clif/pybind11/function_lib.py @@ -214,7 +214,7 @@ def generate_return_value_policy_for_type( for child_param_type in param_type.params: return_value_policy_list.append( generate_return_value_policy_for_type( - child_param_type, is_callable_arg + child_param_type, is_callable_arg, reference_internal ) ) return_value_policy_str = ', '.join(return_value_policy_list) diff --git a/clif/pybind11/generator.py b/clif/pybind11/generator.py index f8969797..9e68a2e3 100644 --- a/clif/pybind11/generator.py +++ b/clif/pybind11/generator.py @@ -43,7 +43,7 @@ def _generate_mangled_name_for_module(full_dotted_module_name: str) -> str: return full_dotted_module_name.replace('_', '__').replace('.', '_') -class ModuleGenerator(object): +class ModuleGenerator: """A class that generates pybind11 bindings code from CLIF ast.""" def __init__(self, ast: ast_pb2.AST, module_path: str, header_path: str, diff --git a/clif/pybind11/variables.py b/clif/pybind11/variables.py index 3a6b5ac6..f80c9990 100644 --- a/clif/pybind11/variables.py +++ b/clif/pybind11/variables.py @@ -33,13 +33,20 @@ def _convert_ptr_to_ref(var_decl: ast_pb2.VarDecl) -> bool: ) -def _generate_self_param_with_type( +def _generate_self_param_type( var_decl: ast_pb2.VarDecl, class_decl: ast_pb2.ClassDecl) -> str: if var_decl.is_extend_variable: assert var_decl.cpp_get.params - return f'{var_decl.cpp_get.params[0].cpp_exact_type} self' - return f'{class_decl.name.cpp_name} &self' + return var_decl.cpp_get.params[0].cpp_exact_type + return f'{class_decl.name.cpp_name}&' + + +def _is_var_decl_type_cpp_copyable(var_decl: ast_pb2.VarDecl) -> bool: + if not var_decl.cpp_get.returns: + return var_decl.type.cpp_copyable + else: + return var_decl.cpp_get.returns[0].type.cpp_copyable def _generate_cpp_get( @@ -65,11 +72,30 @@ def _generate_cpp_get( return_value_policy_pack = ( f'py::return_value_policy_pack({return_value_policy})' ) - self_param_with_type = _generate_self_param_with_type(var_decl, class_decl) + + if not _is_var_decl_type_cpp_copyable(var_decl): + yield from _generate_cpp_get_for_uncopyable_type( + var_decl, class_decl, return_value_policy_pack, ret, + generate_comma=generate_comma) + else: + yield from _generate_cpp_get_for_copyable_type( + var_decl, class_decl, return_value_policy_pack, ret, + generate_comma=generate_comma) + + +def _generate_cpp_get_for_copyable_type( + var_decl: ast_pb2.VarDecl, + class_decl: ast_pb2.ClassDecl, + return_value_policy_pack: str, + ret: str, + generate_comma: bool = False, +) -> Generator[str, None, None]: + """Generate lambda expressions for getters when the type is copyable.""" + self_param_type = _generate_self_param_type(var_decl, class_decl) if generate_comma: - yield I + f'py::cpp_function([]({self_param_with_type}) {{' + yield I + f'py::cpp_function([]({self_param_type} self) {{' else: - yield I + f'[]({self_param_with_type}) {{' + yield I + f'[]({self_param_type} self) {{' yield I + I + f'return {ret};' if generate_comma: yield I + f'}}, {return_value_policy_pack}),' @@ -77,19 +103,49 @@ def _generate_cpp_get( yield I + f'}}, {return_value_policy_pack});' +def _generate_cpp_get_for_uncopyable_type( + var_decl: ast_pb2.VarDecl, + class_decl: ast_pb2.ClassDecl, + return_value_policy_pack: str, + ret: str, + generate_comma: bool = False, +) -> Generator[str, None, None]: + """Generate lambda expressions for getters when the type is uncopyable.""" + if generate_comma: + yield I + 'py::cpp_function([](py::object self_py) -> py::object {' + else: + yield I + '[](py::object self_py) -> py::object {' + self_param_type = _generate_self_param_type(var_decl, class_decl) + yield I + I + f'{self_param_type} self = self_py.cast<{self_param_type}>();' + yield I + I + (f'return py::cast({ret}, {return_value_policy_pack}, ' + 'self_py);') + if generate_comma: + yield I + '}),' + else: + yield I + '});' + + def _generate_cpp_set( var_decl: ast_pb2.VarDecl, class_decl: ast_pb2.ClassDecl, + generate_comma: bool = False, ) -> Generator[str, None, None]: """Generate lambda expressions for setters.""" - yield I + ( - f'[]({class_decl.name.cpp_name}& self, {var_decl.type.cpp_type} v) {{' - ) + if generate_comma: + yield I + (f'py::cpp_function([]({class_decl.name.cpp_name}& self, ' + f'{var_decl.type.cpp_type} v) {{') + else: + yield I + ( + f'[]({class_decl.name.cpp_name}& self, {var_decl.type.cpp_type} v) {{' + ) value = 'v' if var_decl.type.cpp_type.startswith('::std::unique_ptr'): value = 'std::move(v)' yield I + I + f'self.{var_decl.name.cpp_name} = {value};' - yield I + '});' + if generate_comma: + yield I + '}));' + else: + yield I + '});' def generate_from( @@ -107,7 +163,7 @@ def generate_from( if _convert_ptr_to_ref(var_decl): yield f'{class_name}.def_property("{var_decl.name.native}", ' yield from _generate_cpp_get(var_decl, class_decl, generate_comma=True) - yield from _generate_cpp_set(var_decl, class_decl) + yield from _generate_cpp_set(var_decl, class_decl, generate_comma=True) else: return_value_policy = function_lib.generate_return_value_policy_for_type( var_decl.type diff --git a/clif/python/BUILD b/clif/python/BUILD index 0c3e518a..ae1f876c 100644 --- a/clif/python/BUILD +++ b/clif/python/BUILD @@ -21,6 +21,7 @@ cc_library( hdrs = ["pickle_support.h"], visibility = ["//visibility:public"], deps = [ + "@com_google_absl//absl/log:check", "@python_runtime//:python_headers", ], ) diff --git a/clif/python/clif_types.py b/clif/python/clif_types.py index 1c0f0fae..3c90cc1f 100644 --- a/clif/python/clif_types.py +++ b/clif/python/clif_types.py @@ -42,7 +42,7 @@ def Namespace(typedef): return typedef.namespace or '' # Collapse None -> ''. -class TypeDef(object): +class TypeDef: """C++ class as some Python object.""" _genclifuse = False # Generate '// CLIF use' in the header file. diff --git a/clif/python/gen.py b/clif/python/gen.py index ed892e86..5e492a39 100644 --- a/clif/python/gen.py +++ b/clif/python/gen.py @@ -163,7 +163,7 @@ def _DefTable(ctype, cname, lines): yield '};' -class _MethodDef(object): +class _MethodDef: name = 'MethodsStaticAlloc' def __call__(self, methods): @@ -174,7 +174,7 @@ def __call__(self, methods): MethodDef = _MethodDef() # pylint: disable=invalid-name -class _GetSetDef(object): +class _GetSetDef: # pylint: disable=missing-class-docstring name = 'Properties' @@ -968,7 +968,7 @@ def CastAsCapsule(wrapped_cpp, pointer_name, wrapper): yield '}' -class _NewIter(object): +class _NewIter: """Generate the new_iter function.""" name = 'new_iter' @@ -986,7 +986,7 @@ def __call__(self, wrapped_iter, ns, wrapper, wrapper_type): NewIter = _NewIter() # pylint: disable=invalid-name -class _IterNext(object): +class _IterNext: """Generate the iternext function.""" name = 'iternext' diff --git a/clif/python/pickle_support.cc b/clif/python/pickle_support.cc index 81644ca5..224859e4 100644 --- a/clif/python/pickle_support.cc +++ b/clif/python/pickle_support.cc @@ -14,9 +14,44 @@ #include +#include "absl/log/check.h" + namespace clif { +namespace { + +#if PY_VERSION_HEX < 0x030B0000 + +bool ClsGetStateIsBaseObjectGetstate(PyTypeObject*) { return false; } + +#else + +// The object.__getstate__() added with +// https://github.com/python/cpython/pull/2821 +// is not suitable for extension types. +bool ClsGetStateIsBaseObjectGetstate(PyTypeObject* cls) { + PyObject* base_getstate = + PyObject_GetAttrString((PyObject*)&PyBaseObject_Type, "__getstate__"); + CHECK(base_getstate != nullptr); + PyObject* cls_getstate = + PyObject_GetAttrString((PyObject*)cls, "__getstate__"); + if (cls_getstate == nullptr) { + PyErr_Clear(); + Py_DECREF(base_getstate); + return false; + } + bool retval = (cls_getstate == base_getstate); + Py_DECREF(cls_getstate); + Py_DECREF(base_getstate); + return retval; +} + +#endif + +} // namespace + PyObject* ReduceExCore(PyObject* self, const int protocol) { + PyTypeObject* cls = Py_TYPE(self); // Similar to time-tested approach used in Boost.Python: // https://www.boost.org/doc/libs/1_55_0/libs/python/doc/v2/pickle.html // https://github.com/boostorg/python/blob/develop/src/object/pickle_support.cpp @@ -24,15 +59,17 @@ PyObject* ReduceExCore(PyObject* self, const int protocol) { if (getinitargs == nullptr) { PyErr_Clear(); } - PyObject* getstate = PyObject_GetAttrString(self, "__getstate__"); - if (getstate == nullptr) { - PyErr_Clear(); + PyObject* getstate = nullptr; + if (!ClsGetStateIsBaseObjectGetstate(cls)) { + getstate = PyObject_GetAttrString(self, "__getstate__"); + if (getstate == nullptr) { + PyErr_Clear(); + } } PyObject* setstate = PyObject_GetAttrString(self, "__setstate__"); if (setstate == nullptr) { PyErr_Clear(); } - PyTypeObject* cls = Py_TYPE(self); PyObject* initargs_or_empty_tuple = nullptr; PyObject* empty_tuple = nullptr; PyObject* initargs = nullptr; diff --git a/clif/python/primer.md b/clif/python/primer.md index 35e769c3..13f7cbc7 100644 --- a/clif/python/primer.md +++ b/clif/python/primer.md @@ -131,7 +131,7 @@ Bar = ext.Bar @type_customization.extend(ext.Bar): -class _(object): +class _: def foo(self, ...): ... diff --git a/clif/python/pyext.py b/clif/python/pyext.py index 5d77041e..27416c16 100644 --- a/clif/python/pyext.py +++ b/clif/python/pyext.py @@ -94,8 +94,10 @@ _ClassNamespace = lambda pyname: 'py' + pyname # pylint: disable=invalid-name _ITER_KW = '__iter__' +CLIF_MATCHER_VERSION_STAMP_REQUIRED_MINIMUM = 531560963 -class Context(object): + +class Context: """Reflection of C++ [nested] class/struct.""" WRAPPER_CLASS_NAME = 'wrapper' @@ -115,7 +117,7 @@ def _GetCppObj(get='cpp', py='self'): return 'reinterpret_cast<%s*>(%s)->%s' % (WRAPPER_CLASS_NAME, py, get) -class Module(object): +class Module: """Extended context for module namespace.""" def __init__(self, @@ -677,6 +679,17 @@ def GenInitFunction(self, api_source_h): def GenerateBase(self, ast, more_headers): """Extension module generation.""" + if ( + ast.clif_matcher_version_stamp is None + or ast.clif_matcher_version_stamp + < CLIF_MATCHER_VERSION_STAMP_REQUIRED_MINIMUM + ): + raise RuntimeError( + 'Incompatible clif_matcher_version_stamp' + f' ("{ast.clif_matcher_argv0}"):' + f' {ast.clif_matcher_version_stamp} (required minimum:' + f' {CLIF_MATCHER_VERSION_STAMP_REQUIRED_MINIMUM})' + ) ast_manipulations.MoveExtendsBackIntoClassesInPlace(ast) self.init += ast.extra_init for s in gen.Headlines( diff --git a/clif/python/pytd2proto.py b/clif/python/pytd2proto.py index 34e7cbd0..f0f6385f 100644 --- a/clif/python/pytd2proto.py +++ b/clif/python/pytd2proto.py @@ -101,7 +101,7 @@ def _read_include(input_stream, fname, prefix, typetable, capsules, interfaces, continue -class Postprocessor(object): +class Postprocessor: """Process parsed IR.""" def __init__(self, config_headers=None, include_paths=('.',), preamble=''): @@ -927,7 +927,7 @@ def set_func(self, pb, ast): _set_name(p.name, t.name) -class _TypeTable(object): +class _TypeTable: """Manage the Python -> C++ type name mappings. Normally, the Python -> C++ mapping is simple: replace '.' with '::'. However, @@ -1150,7 +1150,7 @@ def __repr__(self): return '_TypeTable(types=\n{types}\n)'.format(types=types) -class _TypeEntry(object): +class _TypeEntry: """Type mapping information for use by _TypeTable. This is private to _TypeTable; _TypeTable has the public APIs for correctly diff --git a/clif/python/type_customization.py b/clif/python/type_customization.py index 9c374360..751c86a6 100644 --- a/clif/python/type_customization.py +++ b/clif/python/type_customization.py @@ -1,7 +1,7 @@ """Decorator for adding attributes (incl. methods) into extension classes.""" -class _EmptyFromClass(object): +class _EmptyFromClass: pass _EMPTY_FROM_CLASS_DICT_KEYS = frozenset(_EmptyFromClass.__dict__) diff --git a/clif/testing/python/extend_methods_test.py b/clif/testing/python/extend_methods_test.py index 61198e37..58de06c4 100644 --- a/clif/testing/python/extend_methods_test.py +++ b/clif/testing/python/extend_methods_test.py @@ -81,12 +81,12 @@ def a_property(self, value): @type_customization.extend(extend_methods.VirtualBaseHolder) -class _(object): +class _: """Added to VirtualBaseHolder.""" @type_customization.extend(extend_methods.ConcreteHolder) -class NamedFromClass(object): +class NamedFromClass: def NfcInstanceMethod(self, prefix): return ':'.join( @@ -109,7 +109,7 @@ def nfc_property(self, value): self.Set(value + 11) -class NfcInstanceMethodOverride(object): +class NfcInstanceMethodOverride: def NfcInstanceMethod(self, prefix): return ':'.join( @@ -149,7 +149,7 @@ class _(_BaseOldStyleFailureTestBase, object): _base_old_style_failure_test_error = None -class _BaseBaseFailureTestBase(object): +class _BaseBaseFailureTestBase: pass diff --git a/clif/testing/python/pickle_compatibility.py b/clif/testing/python/pickle_compatibility.py index 909ccf6f..1b308533 100644 --- a/clif/testing/python/pickle_compatibility.py +++ b/clif/testing/python/pickle_compatibility.py @@ -19,7 +19,7 @@ @type_customization.extend(StoreTwoUsingExtend) # pylint: disable=undefined-variable -class _(object): +class _: def __reduce_ex__(self, protocol): return (self.__class__, (self.Get(0), self.Get(1))) diff --git a/examples/extend_from_python/python/example.py b/examples/extend_from_python/python/example.py index 8ee4a0a0..a082c906 100644 --- a/examples/extend_from_python/python/example.py +++ b/examples/extend_from_python/python/example.py @@ -26,7 +26,7 @@ @type_customization.extend(ext.MyClass) -class _(object): +class _: """Anonymous helper class.""" def g(self):