From 02c8609160cf5b6eecb825a476c3e09249d77a7e Mon Sep 17 00:00:00 2001 From: Chris Rink Date: Tue, 26 Nov 2024 13:37:27 -0500 Subject: [PATCH 01/19] Proxies --- src/basilisp/core.lpy | 136 ++++++++++++++++++++++++++++++++ src/basilisp/lang/interfaces.py | 27 +++++++ 2 files changed, 163 insertions(+) diff --git a/src/basilisp/core.lpy b/src/basilisp/core.lpy index 57a86f4c1..70997fb23 100644 --- a/src/basilisp/core.lpy +++ b/src/basilisp/core.lpy @@ -7,6 +7,7 @@ decimal fractions importlib.util + inspect math multiprocessing os @@ -6779,6 +6780,141 @@ ~@methods) (meta &form)))) +;;;;;;;;;;;;; +;; Proxies ;; +;;;;;;;;;;;;; + +(def ^:private excluded-proxy-methods + #{"__getattribute__" + "__init__" + "__new__"}) + +(def ^:private proxy-cache (atom {})) + +(defn ^:private proxy-base-methods + [base] + (->> (inspect/getmembers base inspect/isroutine) + (remove (comp excluded-proxy-methods first)) + (mapcat (fn [[method-name method]] + (let [meth-sym (symbol method-name) + meth `(fn ~meth-sym [~'self & args#] + (if-let [override (get (.- ~'self ~'-proxy-mappings) ~method-name)] + (apply override ~'self args#) + (-> (.- ~'self __class__) + (python/super ~'self) + (.- ~meth-sym) + (apply args#))))] + [method-name (eval meth *ns*)]))))) + +(defn ^:private proxy-type + "Generate a proxy class with the given bases." + [bases] + (let [methods (apply hash-map (mapcat proxy-base-methods bases)) + base-methods {"__init__" (fn __init__ [self proxy-mappings & args] + (apply (.- (python/super (.- self __class__) self) __init__) args) + (set! (.- self -proxy-mappings) proxy-mappings) + nil) + "_proxy_mappings" nil}] + (python/type (basilisp.lang.util/genname "Proxy") + (python/tuple (concat bases [basilisp.lang.interfaces/IProxy])) + (python/dict (merge methods base-methods))))) + +;; TODO: support object as super +(defn get-proxy-class + "Given one or more base classes, return a proxy class for the given classes. + + Generated classes are cached, such that the same set of base classes will always + return the same resulting proxy class." + [& bases] + (let [base-set (set bases)] + (-> (swap! proxy-cache (fn [cache] + (if (get cache base-set) + cache + (assoc cache base-set (proxy-type bases))))) + (get base-set)))) + +(defn proxy-mappings + "Return the current method map for the given proxy. + + Throws an exception if ``proxy`` is not a proxy." + [proxy] + (if-not (instance? basilisp.lang.interfaces/IProxy proxy) + (throw + (ex-info "Cannot get proxy mappings from object which does not implement IProxy" + {:obj proxy})) + (. proxy (_get-proxy-mappings)))) + +(defn construct-proxy + "Construct an instance of the proxy class ``c`` with the given constructor arguments. + + Throws an exception if ``c`` is not a subclass of + :py:class:`basilisp.lang.interfaces.IProxy`. + + .. note:: + + In JVM Clojure, this function is useful for selecting a specific constructor based + on argument count, but Python does not support multi-arity methods (including + constructors), so this is likely of limited use." + [c & ctor-args] + (if-not (python/issubclass c basilisp.lang.interfaces/IProxy) + (throw + (ex-info "Cannot construct instance of class which is not a subclass of IProxy" + {:class c :args ctor-args})) + (apply c {} ctor-args))) + +(defn init-proxy + "Set the current proxy method map for the given proxy. + + Method maps are maps of string method names to their implementations as Basilisp + functions. + + Throws an exception if ``proxy`` is not a proxy." + [proxy mappings] + (if-not (instance? basilisp.lang.interfaces/IProxy proxy) + (throw + (ex-info "Cannot get proxy mappings from object which does not implement IProxy" + {:obj proxy})) + (do + (. proxy (_set-proxy-mappings mappings)) + proxy))) + +;; TODO: handle nil methods +(defn update-proxy + "Update the current proxy method map for the given proxy. + + Throws an exception if ``proxy`` is not a proxy." + [proxy mappings] + (if-not (instance? basilisp.lang.interfaces/IProxy proxy) + (throw + (ex-info "Cannot update proxy mappings for object which does not implement IProxy" + {:obj proxy})) + (do + (. proxy (_update-proxy-mappings mappings)) + proxy))) + +;; TODO: how to handle multi-arity fns +;; TODO: kwargs on supertypes +(defmacro proxy + [class-and-interfaces args & fs] + (let [formatted-arity (fn [method-name [arg-vec & body]] + [method-name + (apply list method-name (vec (concat ['this] arg-vec)) body)]) + methods (mapcat (fn [[method-name & body]] + (if (vector? (first body)) + [(formatted-arity method-name body)] + (map (partial formatted-arity method-name) body))) + fs)] + `((get-proxy-class ~@class-and-interfaces) {} ~@args))) + +;; TODO: handle explicit class and self +(defmacro proxy-super + "Macro which expands to a call to the method named ``meth`` on the superclass + with the provided ``args``. + + Note that the default" + [meth & args] + `(. (~'python/super) (~meth ~@args))) + ;;;;;;;;;;;;; ;; Records ;; ;;;;;;;;;;;;; diff --git a/src/basilisp/lang/interfaces.py b/src/basilisp/lang/interfaces.py index 9102a8933..0a0abf76b 100644 --- a/src/basilisp/lang/interfaces.py +++ b/src/basilisp/lang/interfaces.py @@ -372,6 +372,33 @@ def pop(self: Self) -> Self: raise NotImplementedError() +class IProxy(ABC): + """``IProxy`` is a marker interface for proxy types. + + All types created by ``proxy`` are automatically marked with ``IProxy``. + + .. seealso:: + + :ref:`proxies`""" + + __slots__ = () + + _proxy_mappings: "IPersistentMap[str, Callable]" + + def _get_proxy_mappings(self) -> "IPersistentMap[str, Callable]": + return self._proxy_mappings + + def _set_proxy_mappings( + self, proxy_mappings: "IPersistentMap[str, Callable]" + ) -> None: + self._proxy_mappings = proxy_mappings + + def _update_proxy_mappings( + self, proxy_mappings: "IPersistentMap[str, Callable]" + ) -> None: + self._proxy_mappings = proxy_mappings + + T_key = TypeVar("T_key") V_contra = TypeVar("V_contra", contravariant=True) From 97d3ea25daded06096e5f56d160ad4412a2b4265 Mon Sep 17 00:00:00 2001 From: Chris Rink Date: Tue, 26 Nov 2024 13:54:15 -0500 Subject: [PATCH 02/19] More --- src/basilisp/core.lpy | 30 ++++++++++++++++++++++++------ src/basilisp/lang/interfaces.py | 6 +++--- 2 files changed, 27 insertions(+), 9 deletions(-) diff --git a/src/basilisp/core.lpy b/src/basilisp/core.lpy index 70997fb23..481c8cd23 100644 --- a/src/basilisp/core.lpy +++ b/src/basilisp/core.lpy @@ -6810,11 +6810,26 @@ "Generate a proxy class with the given bases." [bases] (let [methods (apply hash-map (mapcat proxy-base-methods bases)) - base-methods {"__init__" (fn __init__ [self proxy-mappings & args] - (apply (.- (python/super (.- self __class__) self) __init__) args) - (set! (.- self -proxy-mappings) proxy-mappings) - nil) - "_proxy_mappings" nil}] + base-methods {"__init__" (fn __init__ [self proxy-mappings & args] + (apply (.- (python/super (.- self __class__) self) __init__) args) + (set! (.- self -proxy-mappings) proxy-mappings) + nil) + "_get_proxy_mappings" (fn _get_proxy_mappings [self] + (.- self -proxy-mappings)) + "_set_proxy_mappings" (fn _set_proxy_mappings [self proxy-mappings] + (set! (.- self -proxy-mappings) proxy-mappings) + nil) + "_update_proxy_mappings" (fn _update_proxy_mappings [self proxy-mappings] + (let [updated-mappings (->> proxy-mappings + (reduce* (fn [m [k v]] + (if v + (assoc m k v) + (dissoc m k))) + (.- self -proxy-mappings)) + (persistent!))] + (set! (.- self -proxy-mappings) updated-mappings) + nil)) + "_proxy_mappings" nil}] (python/type (basilisp.lang.util/genname "Proxy") (python/tuple (concat bases [basilisp.lang.interfaces/IProxy])) (python/dict (merge methods base-methods))))) @@ -6878,10 +6893,13 @@ (. proxy (_set-proxy-mappings mappings)) proxy))) -;; TODO: handle nil methods (defn update-proxy "Update the current proxy method map for the given proxy. + Method maps are maps of string method names to their implementations as Basilisp + functions. If ``nil`` is passed in place of a function for a method, that method will + revert to its default behavior. + Throws an exception if ``proxy`` is not a proxy." [proxy mappings] (if-not (instance? basilisp.lang.interfaces/IProxy proxy) diff --git a/src/basilisp/lang/interfaces.py b/src/basilisp/lang/interfaces.py index 0a0abf76b..5a7cd041e 100644 --- a/src/basilisp/lang/interfaces.py +++ b/src/basilisp/lang/interfaces.py @@ -386,17 +386,17 @@ class IProxy(ABC): _proxy_mappings: "IPersistentMap[str, Callable]" def _get_proxy_mappings(self) -> "IPersistentMap[str, Callable]": - return self._proxy_mappings + raise NotImplementedError() def _set_proxy_mappings( self, proxy_mappings: "IPersistentMap[str, Callable]" ) -> None: - self._proxy_mappings = proxy_mappings + raise NotImplementedError() def _update_proxy_mappings( self, proxy_mappings: "IPersistentMap[str, Callable]" ) -> None: - self._proxy_mappings = proxy_mappings + raise NotImplementedError() T_key = TypeVar("T_key") From 206572cc188516c2f5323d0e722e93bab139eecc Mon Sep 17 00:00:00 2001 From: Chris Rink Date: Tue, 26 Nov 2024 13:58:26 -0500 Subject: [PATCH 03/19] Documentation --- docs/pyinterop.rst | 9 ++++++++- src/basilisp/lang/interfaces.py | 4 +++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/docs/pyinterop.rst b/docs/pyinterop.rst index 280e53996..d3fb03a49 100644 --- a/docs/pyinterop.rst +++ b/docs/pyinterop.rst @@ -279,4 +279,11 @@ Users still have the option to use the native :external:py:func:`operator.floord .. seealso:: - :lpy:fn:`quot`, :lpy:fn:`rem`, :lpy:fn:`mod` \ No newline at end of file + :lpy:fn:`quot`, :lpy:fn:`rem`, :lpy:fn:`mod` + +.. _proxies: + +Proxies +------- + +TBD \ No newline at end of file diff --git a/src/basilisp/lang/interfaces.py b/src/basilisp/lang/interfaces.py index 5a7cd041e..79a7d5e1a 100644 --- a/src/basilisp/lang/interfaces.py +++ b/src/basilisp/lang/interfaces.py @@ -379,7 +379,9 @@ class IProxy(ABC): .. seealso:: - :ref:`proxies`""" + :ref:`proxies`, :lpy:fn:`proxy`, :lpy:fn:`proxy-mappings`, :lpy:fn:`proxy-super`, + :lpy:fn:`construct-proxy`, :lpy:fn:`init-proxy`, :lpy:fn:`update-proxy`, + :lpy:fn:`get-proxy-class`""" __slots__ = () From c3993dd97bfe7aae86cc06c65c02b005a7e481d1 Mon Sep 17 00:00:00 2001 From: Chris Rink Date: Tue, 26 Nov 2024 14:03:47 -0500 Subject: [PATCH 04/19] proxy-super --- src/basilisp/core.lpy | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/basilisp/core.lpy b/src/basilisp/core.lpy index 481c8cd23..2ad8c0de4 100644 --- a/src/basilisp/core.lpy +++ b/src/basilisp/core.lpy @@ -6924,14 +6924,14 @@ fs)] `((get-proxy-class ~@class-and-interfaces) {} ~@args))) -;; TODO: handle explicit class and self (defmacro proxy-super "Macro which expands to a call to the method named ``meth`` on the superclass with the provided ``args``. - Note that the default" + Note this macro explicitly captures the implicit ``this`` parameter added to proxy + methods." [meth & args] - `(. (~'python/super) (~meth ~@args))) + `(. (~'python/super (.- ~'this ~'__class__) ~'this) (~meth ~@args))) ;;;;;;;;;;;;; ;; Records ;; From 80fdb139fadaec282b8176fb71dab096e2c3c185 Mon Sep 17 00:00:00 2001 From: Chris Rink Date: Tue, 26 Nov 2024 14:05:07 -0500 Subject: [PATCH 05/19] Transient correctly --- src/basilisp/core.lpy | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/basilisp/core.lpy b/src/basilisp/core.lpy index 2ad8c0de4..ec63fa4bc 100644 --- a/src/basilisp/core.lpy +++ b/src/basilisp/core.lpy @@ -6823,9 +6823,9 @@ (let [updated-mappings (->> proxy-mappings (reduce* (fn [m [k v]] (if v - (assoc m k v) - (dissoc m k))) - (.- self -proxy-mappings)) + (assoc! m k v) + (dissoc! m k))) + (transient (.- self -proxy-mappings))) (persistent!))] (set! (.- self -proxy-mappings) updated-mappings) nil)) From 3578a5ab5d9dc93d40cf8c3ab1a3d8b920e55699 Mon Sep 17 00:00:00 2001 From: Chris Rink Date: Fri, 29 Nov 2024 07:06:48 -0500 Subject: [PATCH 06/19] Changelog --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index baf4344a3..eeb599cc2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Added + * Added support for proxies (#425) + ### Fixed * Fix a bug where `#` characters were not legal in keywords and symbols (#1149) From aed34bd87873f06af34660ebd06a59508e55884c Mon Sep 17 00:00:00 2001 From: Chris Rink Date: Tue, 26 Nov 2024 14:10:29 -0500 Subject: [PATCH 07/19] Update error message --- src/basilisp/core.lpy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/basilisp/core.lpy b/src/basilisp/core.lpy index ec63fa4bc..fd3f29111 100644 --- a/src/basilisp/core.lpy +++ b/src/basilisp/core.lpy @@ -6887,7 +6887,7 @@ [proxy mappings] (if-not (instance? basilisp.lang.interfaces/IProxy proxy) (throw - (ex-info "Cannot get proxy mappings from object which does not implement IProxy" + (ex-info "Cannot set proxy mappings for an object which does not implement IProxy" {:obj proxy})) (do (. proxy (_set-proxy-mappings mappings)) From 3451b4ecb00904d613b562aababebbce8db6a14e Mon Sep 17 00:00:00 2001 From: Chris Rink Date: Tue, 26 Nov 2024 14:20:26 -0500 Subject: [PATCH 08/19] Object base --- src/basilisp/core.lpy | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/basilisp/core.lpy b/src/basilisp/core.lpy index fd3f29111..b9c98c790 100644 --- a/src/basilisp/core.lpy +++ b/src/basilisp/core.lpy @@ -6834,14 +6834,17 @@ (python/tuple (concat bases [basilisp.lang.interfaces/IProxy])) (python/dict (merge methods base-methods))))) -;; TODO: support object as super (defn get-proxy-class - "Given one or more base classes, return a proxy class for the given classes. + "Given zero or more base classes, return a proxy class for the given classes. + + If no classes, Python's ``object`` will be used as the superclass. Generated classes are cached, such that the same set of base classes will always return the same resulting proxy class." [& bases] - (let [base-set (set bases)] + (let [base-set (if (seq bases) + (set bases) + #{python/object})] (-> (swap! proxy-cache (fn [cache] (if (get cache base-set) cache From 81138521990bafb1b9192be2aa68138409e8ea1c Mon Sep 17 00:00:00 2001 From: Chris Rink Date: Tue, 26 Nov 2024 14:38:19 -0500 Subject: [PATCH 09/19] Fix it up --- src/basilisp/core.lpy | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/src/basilisp/core.lpy b/src/basilisp/core.lpy index b9c98c790..330e3f39f 100644 --- a/src/basilisp/core.lpy +++ b/src/basilisp/core.lpy @@ -6829,9 +6829,15 @@ (persistent!))] (set! (.- self -proxy-mappings) updated-mappings) nil)) - "_proxy_mappings" nil}] + "_proxy_mappings" nil} + + ;; Remove Python ``object`` from the bases if it is present to avoid errors + ;; about creating a consistent MRO for the given bases + proxy-bases (concat (remove #{python/object} bases) [basilisp.lang.interfaces/IProxy])] + #_(doseq [[method-name method] methods] + (println method-name method)) (python/type (basilisp.lang.util/genname "Proxy") - (python/tuple (concat bases [basilisp.lang.interfaces/IProxy])) + (python/tuple proxy-bases) (python/dict (merge methods base-methods))))) (defn get-proxy-class @@ -6848,7 +6854,7 @@ (-> (swap! proxy-cache (fn [cache] (if (get cache base-set) cache - (assoc cache base-set (proxy-type bases))))) + (assoc cache base-set (proxy-type base-set))))) (get base-set)))) (defn proxy-mappings @@ -6918,14 +6924,15 @@ (defmacro proxy [class-and-interfaces args & fs] (let [formatted-arity (fn [method-name [arg-vec & body]] - [method-name - (apply list method-name (vec (concat ['this] arg-vec)) body)]) + [(name method-name) + (apply list 'fn method-name (vec (concat ['this] arg-vec)) body)]) methods (mapcat (fn [[method-name & body]] (if (vector? (first body)) - [(formatted-arity method-name body)] + (formatted-arity method-name body) (map (partial formatted-arity method-name) body))) fs)] - `((get-proxy-class ~@class-and-interfaces) {} ~@args))) + #_(println methods) + `((get-proxy-class ~@class-and-interfaces) ~(apply hash-map methods) ~@args))) (defmacro proxy-super "Macro which expands to a call to the method named ``meth`` on the superclass From ea7c1375b18e9263c4b2aeacddb49b671732cc1e Mon Sep 17 00:00:00 2001 From: Chris Rink Date: Wed, 27 Nov 2024 10:41:50 -0500 Subject: [PATCH 10/19] That's an oopsie --- src/basilisp/core.lpy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/basilisp/core.lpy b/src/basilisp/core.lpy index 330e3f39f..da0e2eacf 100644 --- a/src/basilisp/core.lpy +++ b/src/basilisp/core.lpy @@ -6286,7 +6286,7 @@ (let [name-str (name interface-name) method-sigs (->> methods (map (fn [[method-name args docstring]] - [method-name (conj args 'self) docstring])) + [method-name (vec (concat ['self] args)) docstring])) (map #(list 'quote %)))] `(def ~interface-name (gen-interface :name ~name-str From eda91239d624031064199a936fc9a1d3c78fecd5 Mon Sep 17 00:00:00 2001 From: Chris Rink Date: Wed, 27 Nov 2024 11:45:48 -0500 Subject: [PATCH 11/19] More stuff --- src/basilisp/core.lpy | 32 ++++++++++++++++++++--- tests/basilisp/test_proxies.lpy | 45 +++++++++++++++++++++++++++++++++ 2 files changed, 74 insertions(+), 3 deletions(-) create mode 100644 tests/basilisp/test_proxies.lpy diff --git a/src/basilisp/core.lpy b/src/basilisp/core.lpy index da0e2eacf..78b067be6 100644 --- a/src/basilisp/core.lpy +++ b/src/basilisp/core.lpy @@ -6919,17 +6919,43 @@ (. proxy (_update-proxy-mappings mappings)) proxy))) -;; TODO: how to handle multi-arity fns ;; TODO: kwargs on supertypes +;; TODO: check interface/superclass method membership +;; TODO: check if duplicate methods (defmacro proxy + "Create a new proxy class instance. + + The proxy class may implement 0 or more interface (or subclass 0 or more classes), + which are given as the vector ``class-and-interfaces``. If 0 such supertypes are + provided, Python's ``object`` type will be used. + + If the supertype constructors take arguments, those arguments are given in the + potentially empty vector ``args``. + + The remaining forms (if any) should be method overrides for any methods of the + declared classes and interfaces. Not every method needs to be overridden. Override + declarations may be multi-arity to simulate multi-arity methods. Overrides need + not include ``this``, as it will be automatically added and is available within + all proxy methods. Proxy methods may access the proxy superclass using the + :lpy:fn:`proxy-super` macro. + + Overrides take the following form:: + + (single-arity [] + ...) + + (multi-arity + ([] ...) + ([arg1] ...) + ([arg1 & others] ...))" [class-and-interfaces args & fs] (let [formatted-arity (fn [method-name [arg-vec & body]] - [(name method-name) + [(munge method-name) (apply list 'fn method-name (vec (concat ['this] arg-vec)) body)]) methods (mapcat (fn [[method-name & body]] (if (vector? (first body)) (formatted-arity method-name body) - (map (partial formatted-arity method-name) body))) + (mapcat (partial formatted-arity method-name) body))) fs)] #_(println methods) `((get-proxy-class ~@class-and-interfaces) ~(apply hash-map methods) ~@args))) diff --git a/tests/basilisp/test_proxies.lpy b/tests/basilisp/test_proxies.lpy new file mode 100644 index 000000000..376208214 --- /dev/null +++ b/tests/basilisp/test_proxies.lpy @@ -0,0 +1,45 @@ +(ns tests.basilisp.test-proxies + (:require + [basilisp.test :as test :refer [deftest is are testing]])) + +(def no-op-proxy + (proxy [] [])) + +(deftest get-proxy-class-test + (is (identical? (get-proxy-class) (get-proxy-class))) + (is (python/issubclass (get-proxy-class) basilisp.lang.interfaces/IProxy))) + +(deftest proxy-mappings-test + (is (= {} (proxy-mappings no-op-proxy))) + (is (thrown? basilisp.lang.exception/ExceptionInfo + (proxy-mappings (python/object))))) + +(deftest construct-proxy-test + (let [obj-proxy-cls (get-proxy-class)] + (is (instance? obj-proxy-cls (construct-proxy obj-proxy-cls))) + (is (thrown? basilisp.lang.exception/ExceptionInfo + (construct-proxy python/object))))) + +(definterface ToString + (to-string []) + (to-string [arg1]) + (to-string [arg1 & rest])) + +;; TODO: (?) needs to be mutable or setting _proxy_mappings on the instance fails +(deftype ConcreteToString [^:mutable arg] + ToString + (to-string [this] "0") + (to-string [this arg1] (str "1" arg1)) + (to-string [this arg1 & rest] (str "rest" arg1 rest))) + +(deftest proxy-test + (testing "multi-arity interface methods" + (let [p (proxy [ConcreteToString] [1] + (to-string + ([] "hi i am 0") + ([arg1] (str "i am 1 " arg1)) + ([arg1 & args] (str "i am rest " arg1 " " args))))] + (is (= "hi i am 0" (.to-string p))) + (is (= "i am 1 yes" (.to-string p "yes"))) + (is (= "i am rest first " (.to-string p "first"))) + (is (= "i am rest first (:yes)" (.to-string p "first" :yes)))))) From be8674187dedd2d4586ce04006ad418528f2d1d1 Mon Sep 17 00:00:00 2001 From: Chris Rink Date: Wed, 27 Nov 2024 11:55:29 -0500 Subject: [PATCH 12/19] Check it --- src/basilisp/core.lpy | 3 ++- src/basilisp/lang/interfaces.py | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/basilisp/core.lpy b/src/basilisp/core.lpy index 78b067be6..685637f38 100644 --- a/src/basilisp/core.lpy +++ b/src/basilisp/core.lpy @@ -6787,7 +6787,8 @@ (def ^:private excluded-proxy-methods #{"__getattribute__" "__init__" - "__new__"}) + "__new__" + "__subclasshook__"}) (def ^:private proxy-cache (atom {})) diff --git a/src/basilisp/lang/interfaces.py b/src/basilisp/lang/interfaces.py index 79a7d5e1a..91beb1fd4 100644 --- a/src/basilisp/lang/interfaces.py +++ b/src/basilisp/lang/interfaces.py @@ -385,16 +385,17 @@ class IProxy(ABC): __slots__ = () - _proxy_mappings: "IPersistentMap[str, Callable]" - + @abstractmethod def _get_proxy_mappings(self) -> "IPersistentMap[str, Callable]": raise NotImplementedError() + @abstractmethod def _set_proxy_mappings( self, proxy_mappings: "IPersistentMap[str, Callable]" ) -> None: raise NotImplementedError() + @abstractmethod def _update_proxy_mappings( self, proxy_mappings: "IPersistentMap[str, Callable]" ) -> None: From 10f2ef4c657e948a7bdcf76ccba87328cd7d66ac Mon Sep 17 00:00:00 2001 From: Chris Rink Date: Wed, 27 Nov 2024 12:07:15 -0500 Subject: [PATCH 13/19] Fix multi-arity --- src/basilisp/core.lpy | 22 +++++++++++++++------- tests/basilisp/test_proxies.lpy | 1 - 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/src/basilisp/core.lpy b/src/basilisp/core.lpy index 685637f38..6062a5f7c 100644 --- a/src/basilisp/core.lpy +++ b/src/basilisp/core.lpy @@ -6950,14 +6950,22 @@ ([arg1] ...) ([arg1 & others] ...))" [class-and-interfaces args & fs] - (let [formatted-arity (fn [method-name [arg-vec & body]] + (let [formatted-single (fn [method-name [arg-vec & body]] + [(munge method-name) + (apply list 'fn method-name (vec (concat ['this] arg-vec)) body)]) + formatted-multi (fn [method-name & arities] [(munge method-name) - (apply list 'fn method-name (vec (concat ['this] arg-vec)) body)]) - methods (mapcat (fn [[method-name & body]] - (if (vector? (first body)) - (formatted-arity method-name body) - (mapcat (partial formatted-arity method-name) body))) - fs)] + (apply list + 'fn + method-name + (map (fn [[arg-vec & body]] + (apply list (vec (concat ['this] arg-vec)) body)) + arities))]) + methods (mapcat (fn [[method-name & body]] + (if (vector? (first body)) + (formatted-single method-name body) + (apply formatted-multi method-name body))) + fs)] #_(println methods) `((get-proxy-class ~@class-and-interfaces) ~(apply hash-map methods) ~@args))) diff --git a/tests/basilisp/test_proxies.lpy b/tests/basilisp/test_proxies.lpy index 376208214..20761234e 100644 --- a/tests/basilisp/test_proxies.lpy +++ b/tests/basilisp/test_proxies.lpy @@ -41,5 +41,4 @@ ([arg1 & args] (str "i am rest " arg1 " " args))))] (is (= "hi i am 0" (.to-string p))) (is (= "i am 1 yes" (.to-string p "yes"))) - (is (= "i am rest first " (.to-string p "first"))) (is (= "i am rest first (:yes)" (.to-string p "first" :yes)))))) From f2cc90fdbd56516d138c61d0441ebbb8da34d377 Mon Sep 17 00:00:00 2001 From: Chris Rink Date: Wed, 27 Nov 2024 12:15:18 -0500 Subject: [PATCH 14/19] Documentation --- src/basilisp/core.lpy | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/basilisp/core.lpy b/src/basilisp/core.lpy index 6062a5f7c..1a7224f22 100644 --- a/src/basilisp/core.lpy +++ b/src/basilisp/core.lpy @@ -6792,6 +6792,16 @@ (def ^:private proxy-cache (atom {})) +;; One consequence of adhering so closely to the Clojure proxy model is that this +;; style of dispatch method doesn't align well with the Basilisp style of defining +;; multi-arity methods (which involves creating the "main" entrypoint method which +;; dispatches to private implementations for all of the defined arities). +;; +;; Fortunately, since the public interface of even multi-arity methods is a single +;; public method, when callers provide a multi-arity override for such methods, +;; only the public entrypoint method is overridden in the proxy mappings. This +;; should be a sufficient compromise, but it does mean that the superclass arity +;; implementations are never overridden. (defn ^:private proxy-base-methods [base] (->> (inspect/getmembers base inspect/isroutine) @@ -6948,7 +6958,12 @@ (multi-arity ([] ...) ([arg1] ...) - ([arg1 & others] ...))" + ([arg1 & others] ...)) + + .. warning:: + + The ``proxy`` macro does not verify that the provided override implementations + arities match those of the method being overridden." [class-and-interfaces args & fs] (let [formatted-single (fn [method-name [arg-vec & body]] [(munge method-name) From c976092da2ed1e3b181bcc16ffa7137dd2f0b22a Mon Sep 17 00:00:00 2001 From: Chris Rink Date: Wed, 27 Nov 2024 13:17:35 -0500 Subject: [PATCH 15/19] Disallow duplicate method defs in `proxy` macro --- src/basilisp/core.lpy | 38 ++++++++++++++++++++------------- tests/basilisp/test_proxies.lpy | 8 +++++++ 2 files changed, 31 insertions(+), 15 deletions(-) diff --git a/src/basilisp/core.lpy b/src/basilisp/core.lpy index 1a7224f22..6239cc922 100644 --- a/src/basilisp/core.lpy +++ b/src/basilisp/core.lpy @@ -6932,7 +6932,6 @@ ;; TODO: kwargs on supertypes ;; TODO: check interface/superclass method membership -;; TODO: check if duplicate methods (defmacro proxy "Create a new proxy class instance. @@ -6968,21 +6967,30 @@ (let [formatted-single (fn [method-name [arg-vec & body]] [(munge method-name) (apply list 'fn method-name (vec (concat ['this] arg-vec)) body)]) - formatted-multi (fn [method-name & arities] - [(munge method-name) - (apply list - 'fn - method-name - (map (fn [[arg-vec & body]] - (apply list (vec (concat ['this] arg-vec)) body)) - arities))]) - methods (mapcat (fn [[method-name & body]] - (if (vector? (first body)) - (formatted-single method-name body) - (apply formatted-multi method-name body))) - fs)] + formatted-multi (fn [method-name & arities] + [(munge method-name) + (apply list + 'fn + method-name + (map (fn [[arg-vec & body]] + (apply list (vec (concat ['this] arg-vec)) body)) + arities))]) + methods (map (fn [[method-name & body]] + (if (vector? (first body)) + (formatted-single method-name body) + (apply formatted-multi method-name body))) + fs) + method-map (reduce* (fn [m [method-name method]] + (if-let [existing-method (get m method-name)] + (throw + (ex-info "Cannot define proxy class with duplicate method" + {:method-name method-name + :impls [existing-method method]})) + (assoc m method-name method))) + {} + methods)] #_(println methods) - `((get-proxy-class ~@class-and-interfaces) ~(apply hash-map methods) ~@args))) + `((get-proxy-class ~@class-and-interfaces) ~method-map ~@args))) (defmacro proxy-super "Macro which expands to a call to the method named ``meth`` on the superclass diff --git a/tests/basilisp/test_proxies.lpy b/tests/basilisp/test_proxies.lpy index 20761234e..e10683e51 100644 --- a/tests/basilisp/test_proxies.lpy +++ b/tests/basilisp/test_proxies.lpy @@ -20,6 +20,9 @@ (is (thrown? basilisp.lang.exception/ExceptionInfo (construct-proxy python/object))))) +(definterface Describable + (describe-me [])) + (definterface ToString (to-string []) (to-string [arg1]) @@ -33,6 +36,11 @@ (to-string [this arg1 & rest] (str "rest" arg1 rest))) (deftest proxy-test + (testing "disallows duplicate method overrides" + (proxy [Describable] [] + (describe-me [] "I'm a proxy") + (describe-me [] "Proxy"))) + (testing "multi-arity interface methods" (let [p (proxy [ConcreteToString] [1] (to-string From 35e4ef5333c2a7ba083f4a87e25808e3fb547c3a Mon Sep 17 00:00:00 2001 From: Chris Rink Date: Wed, 27 Nov 2024 13:48:58 -0500 Subject: [PATCH 16/19] Sure --- src/basilisp/core.lpy | 42 +++++++++++++++++++++------------ tests/basilisp/test_proxies.lpy | 7 +++--- 2 files changed, 31 insertions(+), 18 deletions(-) diff --git a/src/basilisp/core.lpy b/src/basilisp/core.lpy index 6239cc922..89727377d 100644 --- a/src/basilisp/core.lpy +++ b/src/basilisp/core.lpy @@ -6838,7 +6838,7 @@ (dissoc! m k))) (transient (.- self -proxy-mappings))) (persistent!))] - (set! (.- self -proxy-mappings) updated-mappings) + (. self (_set_proxy_mappings updated-mappings)) nil)) "_proxy_mappings" nil} @@ -6930,7 +6930,6 @@ (. proxy (_update-proxy-mappings mappings)) proxy))) -;; TODO: kwargs on supertypes ;; TODO: check interface/superclass method membership (defmacro proxy "Create a new proxy class instance. @@ -6959,26 +6958,39 @@ ([arg1] ...) ([arg1 & others] ...)) + .. note:: + + Proxy override methods can be defined with Python keyword argument support since + they are just standard Basilisp functions. See :ref:`basilisp_functions_with_kwargs` + for more information. + .. warning:: The ``proxy`` macro does not verify that the provided override implementations arities match those of the method being overridden." [class-and-interfaces args & fs] (let [formatted-single (fn [method-name [arg-vec & body]] - [(munge method-name) - (apply list 'fn method-name (vec (concat ['this] arg-vec)) body)]) + (apply list + 'fn + method-name + (with-meta (vec (concat ['this] arg-vec)) (meta arg-vec)) + body)) formatted-multi (fn [method-name & arities] - [(munge method-name) - (apply list - 'fn - method-name - (map (fn [[arg-vec & body]] - (apply list (vec (concat ['this] arg-vec)) body)) - arities))]) - methods (map (fn [[method-name & body]] - (if (vector? (first body)) - (formatted-single method-name body) - (apply formatted-multi method-name body))) + (apply list + 'fn + method-name + (map (fn [[arg-vec & body]] + (apply list + (with-meta (vec (concat ['this] arg-vec)) (meta arg-vec)) + body)) + arities))) + methods (map (fn [[method-name & body :as form]] + [(munge method-name) + (with-meta + (if (vector? (first body)) + (formatted-single method-name body) + (apply formatted-multi method-name body)) + (meta form))]) fs) method-map (reduce* (fn [m [method-name method]] (if-let [existing-method (get m method-name)] diff --git a/tests/basilisp/test_proxies.lpy b/tests/basilisp/test_proxies.lpy index e10683e51..e9d5edf26 100644 --- a/tests/basilisp/test_proxies.lpy +++ b/tests/basilisp/test_proxies.lpy @@ -37,9 +37,10 @@ (deftest proxy-test (testing "disallows duplicate method overrides" - (proxy [Describable] [] - (describe-me [] "I'm a proxy") - (describe-me [] "Proxy"))) + (is (thrown? basilisp.lang.compiler/CompilerException + (eval '(proxy [Describable] [] + (describe-me [] "I'm a proxy") + (describe-me [] "Proxy")))))) (testing "multi-arity interface methods" (let [p (proxy [ConcreteToString] [1] From 4fa1ccc1f24ac7f201300e85a7d5d1519953384d Mon Sep 17 00:00:00 2001 From: Chris Rink Date: Wed, 27 Nov 2024 14:05:21 -0500 Subject: [PATCH 17/19] Check membership --- src/basilisp/core.lpy | 16 ++++++++++------ tests/basilisp/test_proxies.lpy | 5 +++++ 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/src/basilisp/core.lpy b/src/basilisp/core.lpy index 89727377d..190fee936 100644 --- a/src/basilisp/core.lpy +++ b/src/basilisp/core.lpy @@ -6821,13 +6821,21 @@ "Generate a proxy class with the given bases." [bases] (let [methods (apply hash-map (mapcat proxy-base-methods bases)) + method-names (set (map munge (keys methods))) base-methods {"__init__" (fn __init__ [self proxy-mappings & args] (apply (.- (python/super (.- self __class__) self) __init__) args) - (set! (.- self -proxy-mappings) proxy-mappings) + (. self (_set_proxy_mappings proxy-mappings)) nil) "_get_proxy_mappings" (fn _get_proxy_mappings [self] (.- self -proxy-mappings)) "_set_proxy_mappings" (fn _set_proxy_mappings [self proxy-mappings] + (let [provided-methods (set (keys proxy-mappings))] + (when-not (.issubset provided-methods method-names) + (throw + (ex-info "Proxy override methods must correspond to methods on the declared supertypes" + {:expected method-names + :given provided-methods + :diff (.difference provided-methods method-names)})))) (set! (.- self -proxy-mappings) proxy-mappings) nil) "_update_proxy_mappings" (fn _update_proxy_mappings [self proxy-mappings] @@ -6844,9 +6852,7 @@ ;; Remove Python ``object`` from the bases if it is present to avoid errors ;; about creating a consistent MRO for the given bases - proxy-bases (concat (remove #{python/object} bases) [basilisp.lang.interfaces/IProxy])] - #_(doseq [[method-name method] methods] - (println method-name method)) + proxy-bases (concat (remove #{python/object} bases) [basilisp.lang.interfaces/IProxy])] (python/type (basilisp.lang.util/genname "Proxy") (python/tuple proxy-bases) (python/dict (merge methods base-methods))))) @@ -6930,7 +6936,6 @@ (. proxy (_update-proxy-mappings mappings)) proxy))) -;; TODO: check interface/superclass method membership (defmacro proxy "Create a new proxy class instance. @@ -7001,7 +7006,6 @@ (assoc m method-name method))) {} methods)] - #_(println methods) `((get-proxy-class ~@class-and-interfaces) ~method-map ~@args))) (defmacro proxy-super diff --git a/tests/basilisp/test_proxies.lpy b/tests/basilisp/test_proxies.lpy index e9d5edf26..2dde6af60 100644 --- a/tests/basilisp/test_proxies.lpy +++ b/tests/basilisp/test_proxies.lpy @@ -42,6 +42,11 @@ (describe-me [] "I'm a proxy") (describe-me [] "Proxy")))))) + (testing "disallows overriding non-superclass methods" + (is (thrown? basilisp.lang.exception/ExceptionInfo + (proxy [Describable] [] + (other-method [] "Proxy"))))) + (testing "multi-arity interface methods" (let [p (proxy [ConcreteToString] [1] (to-string From 718e0dad4f65d4762773f391a21d01c40b7c081e Mon Sep 17 00:00:00 2001 From: Chris Rink Date: Fri, 29 Nov 2024 07:14:12 -0500 Subject: [PATCH 18/19] Fix a bug with `case` macro --- CHANGELOG.md | 1 + src/basilisp/core.lpy | 2 +- tests/basilisp/test_core_macros.lpy | 6 ++++++ 3 files changed, 8 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eeb599cc2..38e0e206a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed * Fix a bug where `#` characters were not legal in keywords and symbols (#1149) + * Fix a bug where seqs were not considered valid input for matching clauses of the `case` macro (#1148) ## [v0.3.3] ### Added diff --git a/src/basilisp/core.lpy b/src/basilisp/core.lpy index 190fee936..00db11e60 100644 --- a/src/basilisp/core.lpy +++ b/src/basilisp/core.lpy @@ -3538,7 +3538,7 @@ (mapcat (fn [pair] (let [binding (first pair) expr `(fn [] ~(second pair))] - (if (list? binding) + (if (seq? binding) (map #(vector (list 'quote %) expr) binding) [[(list 'quote binding) expr]]))) (partition 2 diff --git a/tests/basilisp/test_core_macros.lpy b/tests/basilisp/test_core_macros.lpy index f57cbb7b0..d54a405f3 100644 --- a/tests/basilisp/test_core_macros.lpy +++ b/tests/basilisp/test_core_macros.lpy @@ -316,6 +316,11 @@ (let [l #py [1 2 3]] (is (= 6 (areduce l idx ret 0 (+ ret (aget l idx))))))) +(defmacro case-with-seq + [] + (case :val + ~(seq [:val]) :found)) + (deftest case-test (is (= "yes" (case :a :b "nope" :c "mega nope" :a "yes" "nerp"))) (is (= "nope" (case :b :b "nope" :c "mega nope" :a "yes" "nerp"))) @@ -328,6 +333,7 @@ (is (= "also no" (case 'd :a "no" (b c d) "also no" "duh"))) (is (= "duh" (case 'e :a "no" (b c d) "also no" "duh"))) (is (= 0 (case 1 0))) + (is (= :found (case-with-seq))) (let [out* (atom [])] (is (= 2 (case :2 :1 (do (swap! out* conj 1) 1) From 97ce3b0f398324b9f68b3068d7de71df09702613 Mon Sep 17 00:00:00 2001 From: Chris Rink Date: Fri, 29 Nov 2024 07:18:16 -0500 Subject: [PATCH 19/19] Goofed it --- tests/basilisp/test_core_macros.lpy | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/basilisp/test_core_macros.lpy b/tests/basilisp/test_core_macros.lpy index d54a405f3..686859021 100644 --- a/tests/basilisp/test_core_macros.lpy +++ b/tests/basilisp/test_core_macros.lpy @@ -318,8 +318,8 @@ (defmacro case-with-seq [] - (case :val - ~(seq [:val]) :found)) + `(case :val + ~(seq [:val]) :found)) (deftest case-test (is (= "yes" (case :a :b "nope" :c "mega nope" :a "yes" "nerp")))