Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cl/547618069 testing #64

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions clif/pybind11/cindex/extractor.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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):
Expand Down
2 changes: 1 addition & 1 deletion clif/pybind11/function_lib.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion clif/pybind11/generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
78 changes: 67 additions & 11 deletions clif/pybind11/variables.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -65,31 +72,80 @@ 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}),'
else:
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(
Expand All @@ -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
Expand Down
1 change: 1 addition & 0 deletions clif/python/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ cc_library(
hdrs = ["pickle_support.h"],
visibility = ["//visibility:public"],
deps = [
"@com_google_absl//absl/log:check",
"@python_runtime//:python_headers",
],
)
Expand Down
2 changes: 1 addition & 1 deletion clif/python/clif_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
8 changes: 4 additions & 4 deletions clif/python/gen.py
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ def _DefTable(ctype, cname, lines):
yield '};'


class _MethodDef(object):
class _MethodDef:
name = 'MethodsStaticAlloc'

def __call__(self, methods):
Expand All @@ -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'

Expand Down Expand Up @@ -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'

Expand All @@ -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'

Expand Down
45 changes: 41 additions & 4 deletions clif/python/pickle_support.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,25 +14,62 @@

#include <Python.h>

#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
PyObject* getinitargs = PyObject_GetAttrString(self, "__getinitargs__");
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;
Expand Down
2 changes: 1 addition & 1 deletion clif/python/primer.md
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ Bar = ext.Bar


@type_customization.extend(ext.Bar):
class _(object):
class _:

def foo(self, ...):
...
Expand Down
17 changes: 15 additions & 2 deletions clif/python/pyext.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Expand All @@ -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,
Expand Down Expand Up @@ -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(
Expand Down
6 changes: 3 additions & 3 deletions clif/python/pytd2proto.py
Original file line number Diff line number Diff line change
Expand Up @@ -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=''):
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion clif/python/type_customization.py
Original file line number Diff line number Diff line change
@@ -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__)
Expand Down
Loading