[Question] How to handle LazyValues without fearing FunctionUnpackedArgumentsValue | My concerns about UnpackedValues
altrisi opened this issue ยท 5 comments
P.S.: Note that I don't fully understand how to use unpacking, but if what I understood about its behaviour is correct, then the following applies. Sorry if not!
Since unpacking was added to Scarpet, there is a new type of "Value", FunctionUnpackedArgumentsValue
, which extends from ListValue
and is basically a list of arguments basically from some packed arguments. However, that means that it is a Value that can hold more than one actual argument to the function. While this is simple-ish to check when you are just handling Values, if your function requires something to be a LazyValue, there is no safe way to check whether a LazyValue contains an UnpackedValue, or something that is actually waiting to be evaluated.
Therefore, if you have a function similar to lazyFun(Value, Value, Value, LazyValue)
, as far as I understood, you may get an UnpackedValue in those from zero to three, containing all of them or not. That means a lot of extra checking in order to get the position of the rest of values, since else you could get an OutOfBounds, or just values that aren't the expected ones. Now consider the following lazyFun(Value, Value, LazyValue, Value, Value)
. In order to handle unpacked arguments in there, it can be a nightmare. Out the top of my head, this would be the function (update from the future: I got tired and didn't fully finish it, so it resembles more to the first signature):
public lazyFun(List<LazyValue> lv, Context c) {
Value val1, val2, postLazy1, postLazy2;
int consumed;
Value val = lv.get(0).evalValue(c);
if (val instanceof FunctionUnpackedArgumentsValue) {
if (val1.length() > 0) {
val1 = val.getList().get(0);
if (val.length() > 1) {
consumed = 1;
val2 = val1.getList().get(1);
if (val.length() > 2)
throw new InternalExpressionException("Too many arguments in unpacked value before the lazyvalue");
} else {
consumed = 2;
// Not willing to branch another unpacked thing here, but that is, since you have to consume another argument
}
} else {
throw new InternalExpressionException("What? Passed an empty unpacked argument value");
}
} else {
val1 = val;
val = lv.get(1).evalValue(c);
consumed = 2;
if (val instanceof FunctionUnpackedArgumentsValue) {
if (val.length() > 0) {
val2 = val.getList().get(0);
if (val.length() > 1)
throw new InternalExpressionException("Too many arguments in unpacked value before the lazyvalue");
} else {
throw new InternalExpressionException("What? Passed an empty unpacked argument value");
}
} else {
val2 = val;
}
}
LazyValue lazy = lv.get(consumed - 1);
// ... Still missing the Values after the lazy
}
Now, what if you have something that only may be a LazyValue, but could be a regular value? Well, then I don't really know how would you even handle that. Given the signature maybeLazyFun(LazyValue, Value,...)
, the first argument has three different possibilities:
- Being a LazyValue that can actually be evaled
- Being a LazyValue that just contains a "static" Value
- Being a LazyValue that contains a
FunctionUnpackedArgumentsValue
with the "static" value as first element
So, what does that mean? Well, first of all, you can't really check if the lazy value is an UnpackedValue beforehand since that would mean evaluating the LazyValue, which may run something not expected to be ran yet (first case). That means you can't check whether the rest of the arguments to the function are present. You could technically save the result from the first evaluation of the LazyValue if you don't need the variables after it in evaluation and there check if it's an UnpackedValue or not. However, what happens if the LazyValue doesn't have to be evaluated (e.g. a map
function called with an empty list) or the other arguments are required for evaluation? There is no safe way to know if there is an unpacked there to get the rest of the arguments out of.
Current functions using LazyValues would actually probably fail or do unexpected things by receiving something they consider a ListValue (since it extends from it) when passed an Unpacked.
Am I missing something? Or currently functions that require such things need to have explicitly documented that they don't support unpacked things?
P.S.: I consider the AbstractLazyFunction#unpackLazy only as a workaround for the problems outlined here for the functions that don't care about lazy values instead of a solution, since as mentioned, functions requiring those can't really handle them cleanly.
I was going to add more things here after changing the title, but I forgot one of them and couldn't find an example where the other would be a big enough problem.
In your example (two lazy arguments at the end of the evaluated arguments, you could take the notes from the scan
:
List<Value> lv = unpackLazy(llv.subList(0, llv.size()-2), c, Context.NONE);
LazyValue penultimate = llv.get(llv.size()-2);
LazyValue ultimate = llv.get(llv.size()-1);
... continue with life
Oh wow, now I feel dumb for not thinking nor seeing that.
Well, time to remove most of the lazy part of annotated scarpet then.
Well, currently only scan
and volume
from API are mixed, i.e. initial parameters could be eval'ed before the function runs and the last one should never be, since its used as an expression, thus the use of unpack on the range of syntactic params, leaving last one as lazy. Another function like that is 'call' which only evals its arguments when its an actual call, not function signature.
At first I thought that this might be a problem, but after converting everything thought this is really not a big deal - and usecases when unpacking 'may' be useful typically doesn't apply to higher order functions and all sorts of lazy ones (lots of them are unary anyways.
This is little inconsistent, but can be left as an implementation detail. Ideally what this means that functions like (val, val, lazy, val, val) are not really possible, but that's probably a confusing design anyways - most (all?) native functions that expect a lazy, expect it as a last argument, so the handler of the function can unpack unambiguously all the other parameters.
I will add a fine print to the '...' docs operator. I thing all in all - its better to have it (unpacking) than not to have it.
Especially with the API - only in extreme cases you may not need your args expanded and evaluated prior to execution. I knew it may throw a wrench in your annotations PR, but all in all - its should be good. All functions that would use the annotations would be congruent with 'addContextFunction', I don't see a need to complicated it more than for that usecase.
Yeah, it'll be fine, I'll just have to change a few things as to not get lvs but values and remove evaluations. The only thing that really touched lazies were the "converter" to get them directly and optional converter that unpackaged and repackaged them to check if they were null. That'll just kill the first one and simplify the second. I'll still allow returning a lazy cause why not.