Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

splatting into variadic C function yields a BUG #15322

Open
yxhuvud opened this issue Jan 6, 2025 · 7 comments
Open

splatting into variadic C function yields a BUG #15322

yxhuvud opened this issue Jan 6, 2025 · 7 comments
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:compiler:semantic

Comments

@yxhuvud
Copy link
Contributor

yxhuvud commented Jan 6, 2025

Bug Report

Assuming the following definitions:

@[Link("pipewire-0.3")]
lib  LibPipewire
   type Properties = Void
   fun pw_properties_new(key : LibC::Char*, ...) : Properties*
end

Calling

LibPipewire.pw_properties_new(*["foo", "bar", "baz"].map(&.to_unsafe), nil)

yields

Error: BUG: splat expects a tuple, not Array(Pointer(UInt8))

When compiling.

I'd expect that to work, because the whole point of variadic functions is to be able to be called by a variable amount of arguments.

EDIT: Ok so perhaps it shouldn't work, due to general limitations of splatting arrays into functions, but it should probably not emit BUG.

  • Include Crystal compiler version (crystal -v) and OS. If possible, try to see if the bug still reproduces on master.

Crystal 1.14.0 [dacd97b] (2024-10-09)

LLVM: 18.1.6
Default target: x86_64-unknown-linux-gnu
Linux 6.11.0-1007, Ubuntu

@yxhuvud yxhuvud added the kind:bug A bug in the code. Does not apply to documentation, specs, etc. label Jan 6, 2025
@straight-shoota
Copy link
Member

straight-shoota commented Jan 6, 2025

I suppose there are slight differences between variadic concepts, particularly the point of resolution:
In Crystal, variadic calls are entirely resolved at compile time. Calls are fully typed and hence you cannot splat a value where length and element types are determined at runtime (such as Array).
In C however, variadic parameters are only resolved at runtime. You can pass a user-provided string to printf and that'll determine how many arguments the function expects when it executes. Of course this is quite dangerous without precautions, but the language allows it.
Still I don't believe it is possible in C either to call a variadic function with an unknown number of arguments at compile time.

So the behaviour seems to be fine, just the error message needs to be improved.

In some cases the error message for an invalid splat type is not a BUG error:

foo *[1] # Error: argument to splat must be a tuple, not Array(Int32)

@straight-shoota
Copy link
Member

Btw. I get a different error message than reported and this seems correct for the example code. The splatted type is Pointer, not Array.

Error: BUG: splat expects a tuple, not Pointer(Pointer(UInt8))

@yxhuvud
Copy link
Contributor Author

yxhuvud commented Jan 6, 2025

The splatted type is Pointer, not Array.

Ooops, there was a to_unsafe too much in the somewhat simplified example. I updated it.

@ysbaddaden
Copy link
Contributor

The failure is unrelated to C variadic functions: Crystal won't splat anything but a Tuple (length & types known at compile time).

def foo(arg, *rest)
end

foo(*["a", "b", "c"])
 4 | foo(*["a", "b", "c"])
         ^
Error: argument to splat must be a tuple, not Array(String)

Whereas foo(*{"a", "b", "c"}) is accepted. Same if foo was a LibC fun.

AFAIK C behaves the same:

  • each call must have a known number of arguments;
  • we can't create a va_list (only consume from one);
  • we can't pass a va_list to another variadic function (but we can pass a va_list pointer).

https://en.cppreference.com/w/c/variadic/va_list.

@oprypin
Copy link
Member

oprypin commented Jan 6, 2025

I'd like to point out:

  • Something should be done about printing out "BUG" regardless.

  • Even if neither Crystal nor C normally allow to dynamically populate va_args, it can still be a useful feature and there's no technical limitation preventing it. In fact there's already a low-level code snippet that does this.

    call_interface.call(function_pointer, arg_pointers.to_unsafe, pointerof(return_value).as(Void*))

@straight-shoota
Copy link
Member

Yes, the error message needs to be improved. I'm not quite sure why there are two different ones in the first place, or rather what's the difference between argument to splat must be a tuple, not X and BUG: splat expects a tuple, not X.

@ysbaddaden
Copy link
Contributor

I guess it's resolving arguments in a fun vs def.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:compiler:semantic
Projects
None yet
Development

No branches or pull requests

4 participants