clojure-dev

bronsa 2023-08-02T13:08:03.742609Z

just had a quick look at the new dev-methods branch. early comment/question: on the "arity" syntax Klass/meth-2 , what's gonna happen when the class is updated and the method cannot be disambiguated unambiguously? failure or reflection?

fogus (Clojure Team) 2023-08-02T14:26:50.812069Z

two cases: aot code and non-aot. AOT code will have compiled a thunk targeting the method found at the original compile time and so it will just continue to call that. The non-aot case will raise an ambiguity error from the compiler.

bronsa 2023-08-02T13:11:12.377569Z

also, if possible consider keeping the methods dealing with disambiguation public πŸ™‚ would be good to be able to reuse them from tooling (i'm thinking of t.a obv)

fogus (Clojure Team) 2023-08-02T14:27:37.061069Z

I would definitely like to facilitate tooling so maybe some thought can go into what would be useful to expose and how.

bronsa 2023-08-02T14:48:45.356449Z

thank you. when the branch is more in a ready state if you could ping me I can have a deeper look and let you know my thoughts about it

bronsa 2023-08-02T13:59:16.277219Z

and actually another comment :) I see a potential source of bugs when the signature syntax is used within a macro: imagine we have a

(ns foo (:import my.Klass))
(defmacro x [y] `(map Foo/meth-Klass ~y))
because Klass is resolved in the current namespace by the compiler and not by the reader, unlike for type hints, this means that the expansion on the macro implicitely depends on the right import being present at callsites of the macro. so this will fail to resolve Klass
(ns bar (:require foo))
(defn x [] (foo/x [1]))
but more worringly this will resolve to the wrong one
(ns baz (:import my2.Klass) (:require foo))
(defn x [] (foo/x [1])

❀️ 2
fogus (Clojure Team) 2023-08-02T14:27:59.411679Z

Fantastic catch! I'll look into this.

dpsutton 2023-08-02T16:25:25.176489Z

so would this be another source of state in the reader? source cannot read forms with reader resolved keywords, and it’s possible it won’t be able to read functions which use class imports?

dpsutton 2023-08-02T16:25:51.171129Z

(i mean this new shorthand for static methods on imported classes, not in general)

fogus (Clojure Team) 2023-08-02T16:36:56.944759Z

@dpsutton I'm not sure what you mean by "source of state in the reader" -- could you restate the question?

dpsutton 2023-08-02T16:37:39.007979Z

Yes. Give me 15 minutes and I’ll type of core clearly what I mean

πŸ™ 1
fogus (Clojure Team) 2023-08-02T16:38:05.868369Z

@bronsa an open question at the moment is what to do in syntaxQuote if it encounters a signature containing a type that cannot be resolved. I'll noodle on this for a bit.

dpsutton 2023-08-02T16:58:35.025889Z

Ok an example showing what I'm thinking:

(ns foo
  (:require [clojure.set :as set]))

(def x ::set/k)
at a repl, (require 'foo) (source foo/x) results in
user=> (source foo/x)
Execution error at user/eval148 (REPL:1).
Invalid token: ::set/k
This is because the reader expands keywords at read time so needs to know the aliases of the forms it is reading. My understanding was that the solution to the case bronsa showcased here was to have the reader expand the class alias to a fully qualified name. And I'm wondering if this would be another source of state required in the reader to read forms, preventing source from working on any forms which use static methods in this new manner.

bronsa 2023-08-02T17:29:32.970079Z

@fogus I believe it would be reasonable to leave untouched. that's what happens for

`Foo/bar

fogus (Clojure Team) 2023-08-02T17:29:51.499329Z

I'm leaning that way too

bronsa 2023-08-02T17:30:10.229069Z

@dpsutton that's a bug in source , should be of no consideration for this IMO

πŸ™ 1
fogus (Clojure Team) 2023-08-02T17:35:05.841659Z

FWIW: the following is reported by source

(source inspected/mt)
(defn mt [strs]
  (map String/toUpperCase-Locale strs (repeat Locale/ENGLISH)))

fogus (Clojure Team) 2023-08-02T17:35:28.055999Z

I would have been surprised otherwise πŸ™‚

bronsa 2023-08-02T17:36:10.864919Z

yeah autoresolve keywords have a completely different resolution issue than the one proposed here

☝️ 1
πŸ™ 1
bronsa 2023-08-02T14:04:56.792739Z

I believe an acceptable solution would be to extend syntax-quote to resolve those symbols at read time to e.g.

Foo/meth-my.Klass

bronsa 2023-08-02T14:51:08.553429Z

thinking about it a bit more, I believe syntax-quote should be extended to do this regardless, since it already does so for all other interop cases e.g.

user=> `(Double/fromString "")
(java.lang.Double/fromString "")