spy16 / parens

Parens is a highly flexible and embeddable LISP toolkit. :computer:
33 stars 3 forks source link

Implement functional Evaluator, stateful Context and GoExpr #17

Closed lthibault closed 4 years ago

lthibault commented 4 years ago

@spy16 Ok here's what I had in mind. Let me know what you think!

A few notes:

1. Zero-value Evaluator is ready to use

I have removed parens.New in favor of a concrete (non-pointer) Evaluator that can be initialized simply as

var ev parens.Evaluator

The Evaluator handles nil Analyzer and Expander fields by deferring to the built-in implementations, so the zero-value is usable. I really like this because it helps us communicate to users that the Evaluator is stateless.

You'll also notice that all the method receivers are now concrete, as opposed to pointer receivers. I have done this in order to help enforce the statelessness of Evaluator.

Relatedly ...

2. BasicAnalyzer has been exported

Users can implement dynamic dispatch (i.e.: replaceBuiltin=false) by writing a custom analyzer that wraps BasicAnalyzer. It would look a bit like this:


type myAnalyzer struct {
    default parens.Analyzer
}

func (a myAnalyzer)Analyze(ev Evaluator, form value.Any) (Expr, error) {
    // type switch is just an example.  Any custom logic will work.
    switch f := form.(type) {
    case MyType:
        return customLogic(f)
    default:
        return a.default.Analyze(ev, form)
    }
}

3. All state is contained in Context

Because state is inherently complex, I've made Context an interface. The idea is that users should be able to substitute their own implementations, where appropriate. A few hypothetical use-cases:

  1. I'm spawning lots of goroutines that copy very deep stacks, and the call to copy in basicContext.NewChild has become a bottleneck, so I want to replace []StackFrame with a persistent vector/list.
  2. I want to organize my goroutines in something akin to a supervision tree in order to clean up goroutines when their parents stop. To do this, I embed a context.Context in my parens.Context implementation and derive a new context using context.WithCancel on every call to NewChild.

4. GoExpr leverages the fact that Context is passed explicitly

GoExpr.Eval receives the parent Context as its first argument, then does the following:

  1. instantiate a NewChild() context
  2. call the embedded InvokeExpr's Eval method with the child context in a go statement.
lthibault commented 4 years ago

Addendum: this PR implements #12 #14 and most of #16 (special form go is still needed).

spy16 commented 4 years ago

Should i just look at #19 ? That seems to change design of context and how stack is done. Would be easier to review that directly?

lthibault commented 4 years ago

@spy16 Oh whoops, my bad. I mean to post https://github.com/spy16/parens/pull/17#issuecomment-691026995 under #19 πŸ€¦β€β™‚οΈ. (I'm moving the comment to the appropriate location, now. Sorry for the confusion!)

I think it would be easier to start with this one. #19 builds on top of the basic architecture proposed here.

spy16 commented 4 years ago

My main concern right now is the usability/ergonomics of the parens package. I have not clearly understood the benefits of separating Evaluator and Context.. Particularly this snippet:

var ev parens.Evaluator
ctx := parens.NewContext()
fmt.Println(ev.Eval(ctx, "hello"))

Any usage of parens i can think of would always involve starting from a root context. Any child context would be launched as a result of evaluating different expressions (e.g., GoExpr), but except for internal Expr implementations, user wouldn't have to create and manage multiple thread contexts explicitly in Go code.. While it is good to be able to separate out stateless things from the context which is essentially thread state, I am not able to see the benefits over a simpler approach shown below:

// context can be called as Env as well.
rootCtx := parens.New(....)  // functional options can still set custom analyzer, expander, map-factory etc.

rootCtx.Eval(form)

Like you, I am really happy about the how the repl package has turned out and hoping for the same simplicity for this package too. (If we do this effectively, we have a nice and consistent design in all packages, repl.New(), reader.New(), parens.New())..

That said, if this overly complicates something else, or misses out an obvious benefit, we can definitely keep them separate.

lthibault commented 4 years ago

Concerning the fusing fusing Context and Evaluator, their current separation is mostly a side-effect of my hacking around: I was rearranging parts left and right, and this happened to be convenient for this exploratory coding sprint.

In any case, I'm definitely open to merging these two types. I had considered the possibility of implementing namespaces by passing a different Context to the evaluator, but this pushes the responsibility for namespace management up to the REPL, which feels like bad design. I don't have a compelling argument to keep them separate, anymore.

In practical terms, can we merge this in first and then do the refactor in #19? I suspect that embedding Evaluator into Context at this stage will cause conflicts in #19. (In retrospect, I should have managed my branching a bit differently, but I was in "move fast and break things" mode ...)

spy16 commented 4 years ago

"move fast and break things" mode

No worries, it's a good way to get fast feedback and I do this too.. (force push, random commits, multiple near-complete rewrites πŸ˜… )

Do you want me to review this? OR we could just merge it and directly review the other PR and make changes as necessary i think.

lthibault commented 4 years ago

force push, random commits, multiple near-complete rewrites πŸ˜…

lol we adhere to the highest professional standards, here at parens! πŸ˜†

Do you want me to review this? OR we could just merge it and directly review the other PR and make changes as necessary i think.

Let's merge it and review #19. BTW, you'll notice a bit of experimental stuff in that PR as well (namely: the error system). Feel free to aggressively prune things back, if you don't see a clear advantage.