Fork me on GitHub
#cljs-dev
<
2023-08-23
>
dnolen14:08:35

@clojurians.net it is an edgecase for sure. defn is absolutely meant to be top level far as I know.

dnolen14:08:14

there is no acceptable patch - I still think the top level closure is not a good idea. Also people kind of hand waved the Clojure semantics of let and letfn at the namespace level. Like what are the expectations, what are the problems - will there still be caveats etc.

dnolen14:08:23

maybe it’s simple I don’t know - but it’s not properly covered in the issue nor the Ask Clojure question.

gnl15:08:20

@U050B88UR I agree that it's an edgecase, I'm just suggesting that it's a legitimate one – I mentioned some examples in the linked thread. My expectations are pretty straightforward – I would like this...:

(letfn [(f [] ...)
        (g [] ...)]
  (defn h [] (f) (g)))
to behave exactly like this...:
(let [f (fn f [] ...)
      g (fn g [] ...)]
  (defn h [] (f) (g)))
...with the added ability for f to call g. We can certainly disagree on matters of code style, but I hope we can agree that we need to move out of the grey area of "it's probably a bad idea" and into either a) "it's usually a bad idea, but it's supported for the edgecases where it might make sense" or b) "it's not supported and you're on your own". I think the documentation is currently pretty clear about this falling into the a) category. Obviously I'm not a fan of b), but even if things went in that direction, there should at least be a compiler warning/error, rather than the sneaky glitch-in-the-matrix scope-breaking kind of failure we have now (which is made even better by everything working as expected when the top-level letfn is evaluated directly in the REPL). I understand that due to the support for mutual recursion letfn is more than just syntactic sugar for let and that lower level compiler and DCE issues are involved. Do you think that until this is resolved we could have a simple compiler warning on top-level usages of letfn, which links to the issue? Seems like an easy way to make sure noone gets burned until a proper fix is in, which – given the edgecasy nature of it – I can imagine might take a while, and I kinda get it, much as it pains me. :)

gnl15:08:25

...plus a mention in the docs, maybe on the Differences from Clojure page.

gnl15:08:01

Cause this part...: > The following ClojureScript special forms are identical to their Clojure cousins: if, do, let, letfn, quote, loop, recur, throw, and try. ...ain't exactly true right now.

gnl15:08:30

I could open a PR for the documentation.

gnl17:08:00

Just a final note on how common top-level closures with nested defns are. Looking through the results of path:*.clj? /^\(let/ /^[ ]{2,}\(defn-?/ https://github.com/search?q=path%3A*.clj%3F+%2F%5E%5C%28let%2F+%2F%5E%5B+%5D%7B2%2C%7D%5C%28defn-%3F%2F&amp;type=code shows them being used in projects like fulcro, electric, malli, specter, figwheel, timbre, core.typed, quiescent, sicmutils, parsesso, rewrite-clj, membrane, datahike, truss, and many more. Fulcro, sicmutils and parsesso in particular use top-level letfns with many of those usages being in ClojureScript namespaces. If we extend the search to include (do (defn ...) ...), (if ... (defn ...)) and other real-world nested usages of defn, the number of results blows up and includes Clojure/ClojureScript itself. Looking at all this and also taking the already mentioned Clojure documentation into consideration, I'm actually inclined to go back on my initial agreement that any of this is an edgecase – I don't think it is, despite being much less common than the regular top-level usage of defn. I rest my case, prosecution has the floor. :)

gnl19:08:00

P.S. I should perhaps also summarise my reasons for using top-level closures in general, which I suspect other users of them will agree with: a) by acting as local constants of sorts, they can improve performance by letting me evaluate the bound expression only once instead of on every function call, and when that binding is very specific to a particular defn I don't want to pollute the namespace by def-ing it. and b) when the expression is an anonymous function, having it outside the defn makes things much more readable.

tony.kay15:08:32

I’ve used this pattern as well (let around a defn) and I was not the one to come up with it. Pretty sure I picked the idea up out of a clojure book…don’t remember which one, but pretty sure it was a published one. The author was making the point that if you needed something more private than private (didn’t want to pollute the ns itself) that it was arguably better. I look at it from the dynamic language philosophy of clojure (though I know that Clojurescript is just trying to be “close, not exact), and see no philosophical reason for it not to work. I agree that there may be implementation difficulties. They are two different things.

tony.kay15:08:37

At the very least the compile should do one of: • Not accept constructions that will not compile to correct code • Issue a warning that such a construct isn’t supported and should be rewritten • Accept it and provide a working implementation

tony.kay16:08:08

Also, there are tons of examples of emitting let around a def in macro writing. For example page 236 of “Programming Clojure” from May 2009. So, it seems like a reasonable thought process to expect that if something like:

(let [f (fn [...] ...)]
  (def foo (fn [ ... ]  code that uses f)))
works then rewriting that to:
(letfn [(f [...] ...)]
  (defn foo ...))
should also be expected to work

tony.kay16:08:51

In fact I would argue that macros are the go-to case for where top-level let-like closures around defs make sense: I should not be emitting top-level symbols into a ns that are “implementation details” if I can avoid it.

bronsa18:09:17

just FYI, letfn may have different semantics than let [_ (fn when multiple functions are defined, because of the forward declaration of bindings of letfn (by design)

bronsa18:09:45

but yes, in general they should be equivalent modulo forward references

gnl15:09:33

Yup, this was already acknowledged above: > [...] ...with the added ability for f to call g. > I understand that due to the support for mutual recursion letfn is more than just syntactic sugar for let [...]