Request/Feature: give WrenForeignMethodFn userdata or other indication of what function was called

[cxw42]

(This relates to a similar problem described at https://github.com/wren-lang/wren/issues/498#issuecomment-808826754.)

I would like to be able to use one C function for multiple Vala foreign methods. I am working on Vala bindings for Wren. Unless I am missing something, each foreign-method C function receives no information identifying the function being called. Therefore, each function needs its own C callsite. Callsites are hard to create dynamically in compiled languages :) . My current workaround is a fixed list of stub functions I dynamically assign to Wren classes --- functional, but very repetitive and ugly (here et seq.).

I am sure there are multiple ways to accomplish this. One I can think of:

• WrenBindForeignMethodFn returns a structure holding a WrenForeignMethodFn and a void *userdata
• Each WrenForeignMethodFn gets the userdata as a second parameter.

Edit Another way: generate Vala source ahead of time from introspection output! However, I would prefer to be able to bind at runtime. That way Wren could use any Vala class, even classes defined in dynamically-loaded libraries.

What say you? I didn't find an issue for this; apologies if I missed something!

[mhermier]

Considering the current state of the code, this would be the way to go, but will be a huge breaking change.

Personally, I would go another route and make WrenBindForeignMethodFn behave more like a foreign call returning a method object, but this a huge change in the VM with potential performance implications because it adds an extra indirection at call.

[ChayimFriedman2]

I definitely don't know Valva, but for my Rust bindings, I used macros to generate wrappers around the user-provided methods.

[cxw42]

The C++ bindings likewise use templates, which makes sense. Vala unfortunately does not have flexible enough compile-time metaprogramming for that.

this would be the way to go, but will be a huge breaking change.

I don't know if opcodes are a scarce resource - would it be worth retaining the existing code and adding "call foreign method and pass userdata" as a separate operation in the binding? I envision it would be transparent to the Wren code.

make WrenBindForeignMethodFn behave more like a foreign call returning a method object

That does sound very flexible, although (as you noted) at a cost in performance.

[mhermier]

It is not a problem of opcode, since the situation can't be resolved at compile time.

The thing is that your change implies modifying the signature of the foreign method and foreign method binding functions. The internal stuff is pretty straight forward and easy, thought I wonder how costly putting this extra parameter on the stack could be on each foreign call (should be cheap, but need to be bench-marked).

One solution would be to have a secondary method binding function and deprecate the first one, but it I find it mehhhh... and since we are not in stable phase it doesn't really make sence either to bother to preserve the API.

[cxw42]

Working on this at https://github.com/cxw42/wren/tree/wren-vala

[mhermier]

Took a quick look at it, and while your implementation is fine as this, I would have prefered to have userdata as first parameter (but this is a personal taste). That said yours as a potential I only thought now, maybe you can use the make the foreign method signature to use a variadic so it has both signatures at ones (with and without user data), ans so it stays compatible with current API to some degree ? Maybe it is worth it to give it a try at it ???

[cxw42]

Thanks for the quick review!

userdata as first parameter

I kept vm first for consistency with the other API functions. What are some of the benefits of a userdata-first style?

variadic

That is an interesting idea! I have only ever used varargs with a fixed argument that told you what the varargs were, so I'm not sure how that would fit without an API change to distinguish one from two args. Passing both args unconditionally would work fine with the ABIs I know of, but I think it would increase the risk of ABI-related breakage, e.g., when porting to another platform. Have you used this technique before, or can you point me to any examples?

[ChayimFriedman2]

variadic

That is an interesting idea! I have only ever used varargs with a fixed argument that told you what the varargs were, so I'm not sure how that would fit without an API change to distinguish one from two args. Passing both args unconditionally would work fine with the ABIs I know of, but I think it would increase the risk of ABI-related breakage, e.g., when porting to another platform. Have you used this technique before, or can you point me to any examples?

If by "variadic" you mean ..., then it's still a breaking change.

If you mean calling one-argument function with two, this is forbidden in the C standard. It is only allowed for functions that their declaration takes no parameters (ret f(), not ret f(void)), and even with that, this is forbidden in C++ (which Wren aims to be valid on).

I do vote for not breaking existing code, but I am afraid that this will require some complicated infrastructure (like a boolean flag for each foreign method).

[mhermier]

That's the beauty it is transparent. Yon only simply have to call it always with the private data on the calling side, and in the called functions unused parameter will be silently ignored.

int foo(short);
typedef int (*Bar)(short, ...);

int main() {
  Bar bar = &foo; // This is the delicate point that can need a cast, but foo does not need to change
  short baz = 42;
  double bat = 42.0;
  bar(baz, bat); // Here you call it with 2 args, but since foo only take one, only the first one will be used
}

In fact the usage of ... is optional, you can use the complete signature with all the arguments, as long as the casting is achieved.

AFAIK it is not forbiden by the standard but more a UB because of the casting. But anyway I saw this used since so long and it is so useful that even if they want to ban it it would be impossible.

[ChayimFriedman2]

I'm not fan of relying on UB, even when "it can never change". Doing so effectively make Wren non portable.

[mhermier]

Would it makes sense that to avoid these problem in the future we provide macros like we do for built-in but for foreign?

[mhermier]

I don't know if it is too late, but maybe the API should use (u)intptr_t instead of a plain void *. Some users of the API may only require an enum to discriminate the class, and passing a raw pointer as this raise the question of who owns the pointer. Passing uintptr_t makes it clear that the API don't mess with a possible pointer.

[cxw42]

It's not too late, and I like the idea :) . Unfortunately, those types are optional in C99 - https://en.cppreference.com/w/c/types/integer . With void*, we don't need conditional compilation for the type.

I will update the comments in the PR to be more clear that Wren does not own the pointer.

[mhermier]

What do you think to add a typedef void* WrenPrivateData type instead? We start to have a bunch of these private data pointer all around, and they may deserve the same consideration. It would serve as an indication it is not owned by the VM, and would allow to switch to intptr_t in a single line in the future if needed ?

[cxw42]

I like that idea. I would do that in a separate PR, personally, to keep new code and refactoring separate (which I try to do).