mszajna / await-cps

async/await for continuation-passing style functions
MIT License
17 stars 0 forks source link

Support macros expanding to await #2

Open czan opened 5 years ago

czan commented 5 years ago

At the moment async looks through its forms to determine whether each form needs to be transformed into CPS, by looking for the await macro somewhere. Unfortunately, this means that it can't see awaits that get generated by macros.

To provide a concrete use case: at Rokt we use Manifold deferreds to do async IO. We initially wrote something similar to await-cps to rewrite our code into chain calls, but I would like to use await-cps instead. My original thought for how to do that would be to write my own variants of async and await:

(require '[manifold.deferred :as d]
         '[await-cps :as cps])

(defmacro async [& body]
  `(let [d# (d/deferred)]
     (cps/async #(d/success! d# %) #(d/error! d# %)
                ~@body)
     d#))

(defmacro await [d]
  `(cps/await d/on-realized ~d))

But this won't work, because my await won't be macroexpanded into the cps/await call that cps/async is expecting.

mszajna commented 5 years ago

Interoperability is something I definitely want to look at next.

While integrating with lacinia's entirely custom promise implementation I started noticing that await-cps/async is a bit unwieldy in that it's imperative, triggering side effects, not returning a value. There is no value associated with asynchronous execution in await-cps the way there is in manifold or promesa. Because there's no value you can't have functions acting on that value. This induces a proliferation of macros which is obviously not great.

To get back to the realm of values await-cps gives you fn-async which returns a function, essentially (fn [r e] (async r e ...)). That function can be further await-ed in another async block (or another fn-async) or indeed could be invoked manually for interop.

(defn cps->deferred [async-fn]
  (let [d (d/deferred)]
    (async-fn #(d/success! d %) #(d/error! d %))
    d))

(cps->deferred (fn-async [] (:body (await http/get "http://google.com" {:async? true}))))

Perhaps it's unfortunate that the current README stirs users toward async and not fn-async which plays nicer with external code and lends itself better for integration with third party execution libraries. Notably, both C# and JavaScript offer async solely as a modifier on function/method. I intend to update README soon-ish to reflect that. I wouldn't be surprised if async turns out not to be all that useful and maybe eventually deprecated. After all it's equivalent to ((fn-async [] ~@body) respond raise).

I'm also thinking about adding opt-in interop namespaces with cps->* functions and perhaps the other way around too where needed as to not leave the burden down to end user.


So far I have been addressing the former of your macros which, even though I'd approach it differently, actually works perfectly fine on its own. It's the await wrapper that's problematic. The issue is caused by async not fully expanding body before checking for terminator symbols. As your previous PR #1 shows macros are tricky beasts. I would definitely need to get generative testing to cover them before expanding the entire body.

At the same time I think async/await is such a disruptive syntax that I would discourage authors from wrapping awaits anyway. It's enough of a mental overhead looking out for one land mine of a symbol. d/on-realized is the deferred->cps function. Introducing a wrapper, there's not that many keystrokes to be saved and potentially a lot of readability to be lost. That's just my point of view though and I still consider the issue valid. Perhaps just not at the top of my todo list at the moment. I'm still happy to be argued otherwise if you think there's a compelling use-case.

If, however, you're building a manifold specific library where await-cps is a mere implementation detail I'd recommend you use some of the lower level APIs declaring your own terminal symbol (based on do-await). That's obviously not ideal as these can break backward-compatibility without notice but you'd still get the job done with very little own code.

czan commented 5 years ago

I agree with you on the async vs fn-async discussion. Having no value makes async a bit annoying to use, and I have particularly felt that when playing at the REPL. I think the main reason I would be tempted to use async is that fn-async feels a bit clumsy as a name. :stuck_out_tongue:

I can understand your hesitation to introduce this sort of power, given the burden it puts on readers of the code. Macros are one of the mechanisms for abstraction in Clojure, though, so I think there is still general value in having the ability to write macros that expand to await forms. That power can be abused to create poor abstractions, of course, but it can also be used to write good abstractions. Having to put d/on-realized or deferred->cps everywhere has a readability cost, too.

If I were to implement this with appropriate property-based tests, would you be reluctant to merge it on readability/abstraction grounds?

I have considered that I could build my own async for Manifold creating my own terminators, but I don't want to depend on internals of await-cps if I can avoid it.

mszajna commented 5 years ago

The naming is particularly interesting to me - what would be a less clumsy name? I was considering async-fn but I don't quite like def-async-fn, defn-async being bit more concise imo. Otherwise, I could drop the 'fn' bit altogether with defasync and async (in place of fn-async) expecting the user to understand that these macros yield functions.

As I said earlier I would not withhold macroexpansion on the basis of it being too powerful. I want this library to get out of developer's way as much as possible and supporting entirety of Clojure language is vital to achieve this.

I think I actually had an early version macroexpand-all and later a version that did not short-circuit on (not (has-terminators)), greedily transforming all code. In the end I figured it's safer not to transform code that I don't have to. Your latest patch kind of proves it to have been a right call. I have somewhat better confidence in the code as well as the test suite now so I'd be happy to add this feature in given enough coverage.

czan commented 5 years ago

I think it feels clumsy for two reasons: firstly because it's significantly longer than fn, and secondly because the "async" part is an adjective which usually comes before the noun it's modifying (eg. total function, partial function, async function). How do you feel about the name afn?

There are also vague thoughts in the back of my head about whether await is the right primitive to use here. Something more akin to shift (in the delimited continuations sense) or call/cc is really the primitive, and the current await implementation could be written on top of that. I'm not actively working towards that, but if things move in that direction then macros would become even more useful.

I'm going to try building out some more property-based tests, then I'll see about implementing more invasive macroexpansion.

mszajna commented 5 years ago

afn sounds pretty good actually. What would be the var definition counterpart though? deafn has an odd ring to it :smile:. defafn has the double 'f' that defn so smoothly avoids. def-afn perhaps?

Regarding shift and call/cc - I agree there's something more primitive out there that you could define async/await around and it's definitely a valid argument in favour of macros. At the same time, this library's purpose is to provide async/await syntax similar to JS or C#'s with the assumption that it is familiar to Clojure developers coming from Java and JS.

Perhaps there is a more generic library to be extracted from this code base that would appeal to true Lispers better but I wouldn't want these low level primitives in the public API of await-cps.

czan commented 5 years ago

I'm not convinced that the symmetry of fn/defn needs to be maintained. I think afn/defn-async is a reasonable combination. Given that defn is almost always used at the top-level, having a slightly longer name is more reasonable.

mszajna commented 5 years ago

It doesn't need to be maintained but it would have been less of a cognitive overhead. Also, I assume the argument about fn-async's clumsiness applies to defn-async just as well. But I'm leaning towards afn/defn-async anyway. Inline functions need to be concise. See #3. Thanks for your help with naming and sorry for hijacking this issue.