Closed bwo closed 9 years ago
Nice! Yeah, this was exactly how I imagined it looking.
The whole "UnEvalFn taking eval
" as a parameter is just temporary. Once the Context work lands, instead of taking an Env and an Eval function, it'll just take a LoadedContext.
I have a PR that I just got working here that completely redoes the whole FFI implementation and interface: https://github.com/TyOverby/ares/pull/9
I have yet to unify it with the Context based work, but it simplifies the FFI code a lot. I think it'll work out really well with your apply
implementation.
I'm not sure how I feel about having a deeper hierarchy for functions.
Right now we have
Value
Lambda
ForeignFunction
UnEvalFn
FreeFn
And I did work in #9 to make it look like
Value
Lambda
ForeignFunction // calling free_fn() or ast_fn() generates one of these.
I just don't see much benefit to making it deeper than it needs to be.
Yeah, the only real reason for that was to (a) somewhat segregate the check for "do we need to evaluate arguments?" from the work of function application and (b) to ensure that if you're calling apply
you have something that can be apply
-ed. But neither of those is actually a big deal.
I merged the apply
changes in 00e86ea09ea863d70ccbc2433d47c26ea603f22e; thanks!
I might revisit the idea of consolodating under Function again some time, but right now I'm happy with returning Err(NotExecutable).
also separates out functions into a single variant in Value
so something like this. Not sure this reduces the number of clones overall, actually, but I wanted to put up an example of what I was thinking of.
I opted to just pull in
apply
directly in list.rs, partly to avoid having to change the signature of everyUnEvalFn
to accept an additional parameter and partly because I wasn't certain what the rationale for passingeval
in as an argument was.