Change function pointers to std::function

MoTrix

I've been thinking of replacing the function pointer system in miniscript2 used for text output and intrinsic code to instead be std::function, in order to make the API easier to use with lambda captures--which already mirrors the delegate system used in the C# version more accurately and is not a hard change--just a modification of the typedefs in the transpiler comments, and std::function isn't really any slower than function pointers anyway.
It's still up to you. But i wanna hear your thoughts on the proposal before making a PR that might get rejected if submitted without permission.

jstrout

It's not a terrible idea. There are two reasons I used function pointers in the first place, and they're the same reasons I tend to reach for C solutions rather than STL solutions when they do the same job:

I want to avoid dragging in too much of the STL, which can bloat an app and make it harder to run on embedded systems, for example.
I have the vague idea of providing C bindings, to make it easier to embed in languages like Python (or anywhere that supports SWIG). So the less translation/wrapping we have to do for that, the better.

Both of these are more "nice to have" than "must have". Just so I fully understand the motivation, can you give a concrete example or two of how std::function would make your life easier?

MoTrix

Well, for example, when dealing with multiple interpreters of an unknown amount, such as ones based on an arbitrary variable, it would be nice to be able to be able to add unique text outputs for each runtime-created interpreter to make each one have a unique I/O, like concatenating a "name" or "id" before the output. If you were to use function pointers, it would be close to impossible to determine which interpreter has that specific text output lambda without being able to capture unique information about it. Best you can do is probably hardcoded function pointers in an array that print a different prefix each, but those would need to be manually written and might not be enough if you end up with a higher number of interpreters.
Another thing is if you wanted to change intrinsic code in runtime which allows you to dynamically change their functionality by capturing a string or enum combined with an if or switch statement. Which would actually be an interesting way to change the "mode" of an intrinsic or generating new ones altogether based on specific calls, without having to make the user pass something as an argument each time.
In short, it's mainly for runtime code generation, which is mainly relevant to me wanting to use a secondary scripting language to be able to interact with the MiniScript API--and the current function pointer approach is making it difficult. But I'm fine with keeping it this way, this is definitely more of a "nice to have" and for your goals it will probably be better to keep it as-is. I can just make my own fork for such runtime-heavy projects. 😃

Edwards

MoTrix Yeah, that’s a reasonable use case for closures—being able to capture per-interpreter state would make dynamically generated runtimes much easier to manage. For a simpler API, though, I can see why function pointers were kept.

jstrout

Yeah that makes sense. Please go ahead and submit a PR, and I'll make a final decision after studying the code. But it sounds like a change worth making.

MoTrix

I submitted the PR. The change turned out to be minimal (only 4 lines)--and had no regressions.

jstrout

It's a reasonable request. Thanks for the PR — now merged. 🙂

MoTrix

Thanks for merging!