redplanetlabs / proxy-plus

A replacement for Clojure's proxy that's 10x faster and more usable
Apache License 2.0
175 stars 10 forks source link

Tricky Edge Cases #20

Open owenRiddy opened 1 year ago

owenRiddy commented 1 year ago

Hi again. I have detected a tricky edge case. I'm don't know Java very well, so bear with me if I get something wrong here:

public class TestBaseClass3{
    public int trickyCase(int a, String b) {
        return 6;
    }

    public int trickyCase(java.lang.Integer a, String b) {
        return 7;
    }
}

It is trickey to override the trickyCase that returns 6.

(proxy+ [] TestBaseClass3
                  ;; Impossible: "Only long and double primitives are supported"
                   (trickyCase [this ^int a b] 8))

I propose that there should be an option to explicitly declare the type signatures independent of hints. I found something like (trickyCase [this a b] [int java.lang.String :=> int] ...) ergonomic (example: https://github.com/owenRiddy/proxy-plus-minus/blob/main/test/clj/proxy_plus_minus/core_test.clj#L276).

If you like the idea I can put together a PR for proxy-plus.

nathanmarz commented 1 year ago

Is the "Only long and double primitives" error coming from the Clojure compiler?

owenRiddy commented 1 year ago

Yes. The fine print says

Functions have limited support for primitive arguments and return type: type hints for long and double (only these) generate primitive-typed overloads. Note that this capability is restricted to functions of arity no greater than 4.

I was running in to that while proxying a javax.swing.table.DefaultTableModel. In 80% of cases people could probably push through and find something that works, but to me the obvious thing was to separate type hints for proxy+ and type hints for the Clojure compiler.

nathanmarz commented 1 year ago

Looking at the code, I think the method matching is fine. It's just the Clojure fns are invalid due to the type hints. Would a patch that strips unsupported type hints, but still uses them for method matching, fix this?

owenRiddy commented 1 year ago

I think that would be enough. Problems were only occurring at compile time and I didn't see anything complaining apart from the Clojure compiler.

nathanmarz commented 1 year ago

Happy to accept a PR for this