Regarding State::loadFunction and Value::ProcType

[27justin]

Hi everyone.

After playing around with ArkScript for a bit in the recent past, I've noticed that the function argument in State::loadFunction is a Value(*)(std::vector<Value> &, VM *).

Is there any specific reason for not using std::function<> for this? Using the bare C function pointer prevents the use of using lambda captures, which especially for this use-case I would deem important.

[SuperFola]

Hi!

If I remember correctly, at the time it was to limit the size of the variant held by Value.

AFAICT a std::function is 32 bytes, same as std::string, so it should not have any impact if we were to switch the function pointer to a std::function! (for a while we were using a custom smaller String, that was about 24 bytes iirc, hence the "no std function")

I'll see if changing that brings performances up/down/neutral, that shouldn't break anything other than that

[27justin]

I hacked together a local version of Ark that is compatible with std::function<>, though I'm not familiar with the benchmark/test-suite, so I can't check whether it passes the tests.

This particularly was an effort to implement a templated loadFunction that directly takes a lambda with the intended types instead of pulling them from the vector, let me know if you want a pull request or anything, would be happy to collaborate.

  state.loadFunction("hello-world", [&](std::string name, int age) {
    std::cout << "Hello " << name << ", age: " << age << "\n";
    return 99;
  });

[SuperFola]

I'd be interested in a PR!

It can be easier to implement if State.loadFunction takes a std::function<Value(std::vector<Value>&, VM&), though harder to use for the end user (modules suffer from the same problem, but as they are more low level it isn't really a problem IMO).

[27justin]

Sweet, I'll clean the code up on my end and submit a PR within due time.

Regarding loadFunction, I was thinking we could overload it to accept both the generic and the parameter-specialized variants. That said, since the deserialization of Ark::Value into arguments is a bit tricky and likely needs more careful thought, I’ll hold off on including that in the initial PR.
If you don't mind, I can submit a separate PR for it once the std::function lands.