terralang / terra

Terra is a low-level system programming language that is embedded in and meta-programmed by the Lua programming language.
terralang.org
Other
2.72k stars 201 forks source link

Allow vararg function definitions #443

Closed ErikMcClure closed 4 years ago

ErikMcClure commented 4 years ago

It turns out that Terra can actually forward varargs if you allow vararg function definitions, by using LLVM's built-in va_list intrinsics. Here is an example of a logging function:

local va_start = terralib.intrinsic("llvm.va_start", {&int8} -> {})
local va_end = terralib.intrinsic("llvm.va_end", {&int8} -> {})

terra Log(level : int, f : rawstring, ...) : {}
  if level >= 0 then
    C.printf(Levels[level])
  end
  var vl : C.va_list
  va_start([&int8](&vl))
  C.vprintf(f, vl)
  va_end([&int8](&vl))
  C.printf("\n");
end

It may be possible to extract individual arguments out, but I haven't attempted to use LLVM's va_arg intrinsic, and Terra might not like it. This change simply forwards the varargs state to the function definition.

aiverson commented 4 years ago

I don't like needing to use va_start and va_end like this is C. Terra is a higher level language, it should be able to use ... to access varargs.

Having a function in terralib that turns varargs into an iterable list for consuming in a for loop would be nice, and we should be able to just use ... to forward varargs to another function.

elliottslaughter commented 4 years ago

@aiverson I think these are good points and a starting point for future discussion on what a truly "Terra native" varargs support would look like. On the other hand, being able to support C ABI compatibility is also useful, so I think this patch is worthwhile.

@ErikMcClure could you add a test like what you have in the issue description? I want to make sure we actually test this behavior, if we want to support it.

ErikMcClure commented 4 years ago

@elliottslaughter I've added a test that uses a C function to iterate through and validate each example argument using va_arg. This only tests passing through the va_list parameter.

elliottslaughter commented 4 years ago

Looks like llvm.org must have been down, but I plan to merge this. (Will try to restart the tests first, but if it's still down I'll just merge.)

elliottslaughter commented 4 years ago

Ok, whatever. llvm.org is still down. I'm just going to merge this.