From 5bfad7439abe784aaa737ad9aeb939ea6c8590ab Mon Sep 17 00:00:00 2001 From: Eric Cousineau Date: Mon, 29 Jan 2018 16:04:27 -0500 Subject: [PATCH] cpp_const: Add ability to impose C++ const semantics in Python. --- bindings/pydrake/BUILD.bazel | 1 + bindings/pydrake/third_party/BUILD.bazel | 76 +++++ bindings/pydrake/third_party/__init__.py | 1 + .../pydrake/third_party/forward_files.bzl | 35 +++ bindings/pydrake/util/BUILD.bazel | 17 ++ bindings/pydrake/util/cpp_const.py | 280 ++++++++++++++++++ bindings/pydrake/util/test/cpp_const_test.py | 146 +++++++++ 7 files changed, 556 insertions(+) create mode 100644 bindings/pydrake/third_party/BUILD.bazel create mode 100644 bindings/pydrake/third_party/__init__.py create mode 100644 bindings/pydrake/third_party/forward_files.bzl create mode 100644 bindings/pydrake/util/cpp_const.py create mode 100644 bindings/pydrake/util/test/cpp_const_test.py diff --git a/bindings/pydrake/BUILD.bazel b/bindings/pydrake/BUILD.bazel index d51696c2802f..e8be00a0f69e 100644 --- a/bindings/pydrake/BUILD.bazel +++ b/bindings/pydrake/BUILD.bazel @@ -135,6 +135,7 @@ PYBIND_LIBRARIES = adjust_labels_for_drake_hoist([ "//drake/bindings/pydrake/examples", "//drake/bindings/pydrake/solvers", "//drake/bindings/pydrake/systems", + "//drake/bindings/pydrake/third_party", "//drake/bindings/pydrake/util", ]) diff --git a/bindings/pydrake/third_party/BUILD.bazel b/bindings/pydrake/third_party/BUILD.bazel new file mode 100644 index 000000000000..6b169a17a09e --- /dev/null +++ b/bindings/pydrake/third_party/BUILD.bazel @@ -0,0 +1,76 @@ +load("@drake//tools/install:install.bzl", "install") +load("//tools/lint:lint.bzl", "add_lint_tests") +load( + "//tools/skylark:drake_py.bzl", + "drake_py_library", +) +load( + "//tools/skylark:pybind.bzl", + "get_pybind_package_info", +) +load(":forward_files.bzl", "forward_files") + +package(default_visibility = [ + "//bindings/pydrake:__subpackages__", +]) + +# This determines how `PYTHONPATH` is configured, and how to install the +# bindings. +PACKAGE_INFO = get_pybind_package_info( + base_package = "//bindings", +) + +drake_py_library( + name = "module_py", + srcs = ["__init__.py"], + imports = PACKAGE_INFO.py_imports, +) + +# Forward physical files to simplify working with `third_party` files (both +# for Python imports and for installation). +wrapt_prefix = "//third_party:com_github_grahamdumpleton_wrapt/" + +forward_files( + srcs = [wrapt_prefix + f for f in [ + "__init__.py", + "LICENSE", + "wrappers.py", + ]], + dest_prefix = "wrapt/", + strip_prefix = wrapt_prefix, + tags = ["nolint"], + visibility = ["//visibility:private"], +) + +# TODO(eric.cousineau): Use Ubuntu packages once we have a later version of +# `python-wrapt` that does not have bugs like a mis-mapped `__mod__` proxy. +drake_py_library( + name = "wrapt_py", + srcs = [ + "wrapt/__init__.py", + "wrapt/wrappers.py", + ], + imports = PACKAGE_INFO.py_imports, + tags = ["nolint"], +) + +PY_LIBRARIES = [ + ":module_py", + ":wrapt_py", +] + +drake_py_library( + name = "third_party", + visibility = ["//visibility:public"], + deps = PY_LIBRARIES, +) + +install( + name = "install", + targets = PY_LIBRARIES, + py_dest = PACKAGE_INFO.py_dest, + docs = ["wrapt/LICENSE"], + doc_dest = "share/doc", +) + +add_lint_tests() diff --git a/bindings/pydrake/third_party/__init__.py b/bindings/pydrake/third_party/__init__.py new file mode 100644 index 000000000000..c375e238d2e4 --- /dev/null +++ b/bindings/pydrake/third_party/__init__.py @@ -0,0 +1 @@ +# Empty Python module. diff --git a/bindings/pydrake/third_party/forward_files.bzl b/bindings/pydrake/third_party/forward_files.bzl new file mode 100644 index 000000000000..0680adf6f23a --- /dev/null +++ b/bindings/pydrake/third_party/forward_files.bzl @@ -0,0 +1,35 @@ +# -*- python -*- + +def forward_files( + srcs, + strip_prefix, + dest_prefix, + visibility = None, + tags = []): + """Forwards files in `srcs` to be physically present in the current + packages. + + Present implementation simply copies the files. + + @param srcs + List of string, pointing *directly* to files. Does not handle filegroup + targets. + @param strip_prefix + String to be stripped from source files. Should include trailing slash. + @param dest_prefix + String to be prepended to target. + """ + outs = [] + for src in srcs: + if not src.startswith(strip_prefix): + fail("'{}' not under '{}'".format(src, strip_prefix)) + out = dest_prefix + src[len(strip_prefix):] + native.genrule( + name = out + ".forward", + srcs = [src], + outs = [out], + cmd = "cp $< $@", + tags = tags, + visibility = visibility, + ) + outs.append(out) diff --git a/bindings/pydrake/util/BUILD.bazel b/bindings/pydrake/util/BUILD.bazel index 71f716541f8d..1269cd2bfed6 100644 --- a/bindings/pydrake/util/BUILD.bazel +++ b/bindings/pydrake/util/BUILD.bazel @@ -98,6 +98,15 @@ drake_cc_library( ], ) +drake_py_library( + name = "cpp_const_py", + srcs = ["cpp_const.py"], + deps = [ + ":module_py", + "//bindings/pydrake/third_party:wrapt_py", + ], +) + drake_cc_library( name = "drake_optional_pybind", hdrs = ["drake_optional_pybind.h"], @@ -106,6 +115,7 @@ drake_cc_library( PYBIND_LIBRARIES = [] PY_LIBRARIES = [ + ":cpp_const_py", ":cpp_param_py", ":cpp_template_py", ":module_py", @@ -160,6 +170,13 @@ drake_py_test( ], ) +drake_py_test( + name = "cpp_const_test", + deps = [ + ":cpp_const_py", + ], +) + drake_py_test( name = "cpp_param_test", deps = [ diff --git a/bindings/pydrake/util/cpp_const.py b/bindings/pydrake/util/cpp_const.py new file mode 100644 index 000000000000..2956cee1911a --- /dev/null +++ b/bindings/pydrake/util/cpp_const.py @@ -0,0 +1,280 @@ +""" +Enables a semi-transparent layer of C++ const-honoring Python proxies. +""" + +import inspect +from types import MethodType +from pydrake.third_party.wrapt import ObjectProxy + +# TODO(eric.cousineau): Add mechanism for enabling / disabling const-proxying. + +# TODO(eric.cousineau): Determine if propagation for `const` attributes and +# methods is ever useful in pure Python, for things such as `__iter__`, +# `__getitem__`, `__getslice__`, etc. + + +class ConstError(TypeError): + """Indicates `const` access violations.""" + pass + + +class _ConstClassMeta(object): + # Provides metadata for a given class. + def __init__(self, cls, owned_properties=None, mutable_methods=None): + self._cls = cls + self._owned_properties = set(owned_properties or set()) + self.mutable_methods = set(mutable_methods or set()) + # Add any decorated mutable methods. + methods = inspect.getmembers(cls, predicate=inspect.ismethod) + for name, method in methods: + # TODO(eric.cousineau): Warn if there is a mix of mutable and + # immutable methods with the same name. + if getattr(method, '_is_mutable_method', False): + self.mutable_methods.add(name) + # Handle inheritance. + for base_cls in self._cls.__bases__: + base_meta = _const_metas.get(base_cls) # Minor cyclic dependency. + self._owned_properties.update(base_meta._owned_properties) + self.mutable_methods.update(base_meta.mutable_methods) + + def is_owned_property(self, name): + # Determines if a property is owned, and if the returned value should + # be `const`. + return name in self._owned_properties + + def is_mutable_method(self, name): + # Determines if a method is mutable by name. + # N.B. This would not handle overloads (e.g. differing between + # `T& get()` and `const T& get() const`). + # However, if using `pybind11`, the method signatures should play well + # with custom `type_caster`s that will permit the overloads to + # organically prevent `const` violations. + return name in self.mutable_methods + + +class _ConstClassMetaMap(object): + # Provides mapping from class to metadata. + def __init__(self): + self._meta_map = {} + + def emplace(self, cls, owned_properties=None, mutable_methods=None): + # Constructs metadata and reigstered it. + meta = _ConstClassMeta(cls, owned_properties, mutable_methods) + return self._add(cls, meta) + + def _add(self, cls, meta): + assert cls not in self._meta_map + self._meta_map[cls] = meta + return meta + + def get(self, cls): + # Retrieves metadata for a class. + # Assumes class has not changed if it's already registered. + meta = self._meta_map.get(cls, None) + if meta: + return meta + else: + # Construct default. + return self._add(cls, _ConstClassMeta(cls)) + + +_const_metas = _ConstClassMetaMap() +# Register common mutators. +# N.B. All methods registered will respect inheritance. +# N.B. For `object`, see `_install_object_mutable_methods`. +_const_metas.emplace(object, mutable_methods={ + "__setattr__", + "__delattr__", + "__setitem__", + "__delitem__", + "__setslice__", + "__iadd__", + "__isub__", + "__imul__", + "__idiv__", + "__itruediv__", + "__ifloordiv__", + "__imod__", + "__ipow__", + "__ilshift__", + "__irshift__", + "__iand__", + "__ixor__", + "__ior__", + "__enter__", + "__exit__", + }) +_const_metas.emplace(list, mutable_methods={ + "append", + "clear", + "extend", + "insert", + "pop", + "remove", + "reverse", + "sort", + }) +_const_metas.emplace(dict, mutable_methods={ + "clear", + "pop", + "popitem", + "setdefault", + "update", + }) + + +class _Const(ObjectProxy): + # Wraps an object, restricting access to non-mutable methods and + # properties. + def __init__(self, wrapped): + ObjectProxy.__init__(self, wrapped) + + def __getattr__(self, name): + # Intercepts access to mutable methods, general methods, and owned + # properties. + wrapped = object.__getattribute__(self, '__wrapped__') + meta = _const_metas.get(type(wrapped)) + value = getattr(wrapped, name) + if meta.is_mutable_method(name): + # If decorated as a mutable method, raise an error. Do not allow + # access to the bound method, because the only way this method + # *should* become usable is to rebind it. + _raise_mutable_method_error(self, name) + elif _is_method_of(value, wrapped): + # Rebind method to const `self` recursively catch basic violations. + return _rebind_method(value, self) + elif meta.is_owned_property(name): + # References (pointer-like things) should not be const, but + # internal values should. + return to_const(value) + else: + return value + + def __dict_custom__(self): + return to_const(self.__wrapped__.__dict__) + + def __repr__(self): + out = self.__wrapped__.__repr__() + if (len(out) >= 2 and out[0] == '<' and out[-1] == '>'): + return '