Skip to content
This repository was archived by the owner on Aug 31, 2021. It is now read-only.

[[ LCB ]] Add varargs support to foreign handlers#5666

Merged
runrevmark merged 4 commits into
livecode:developfrom
runrevmark:ffi-varargs
Jul 7, 2017
Merged

[[ LCB ]] Add varargs support to foreign handlers#5666
runrevmark merged 4 commits into
livecode:developfrom
runrevmark:ffi-varargs

Conversation

@runrevmark

Copy link
Copy Markdown
Contributor

This patch adds varargs support to foreign C handlers, allowing
binding to a wider range of C functions.

The syntax for vararg handlers is similar to that of C:

  foreign handler myvarargs(in pFirst as Pointer, ...) returns CInt binds to ...

Using the token ... to indicate the start of the variable
parameter list.

To implement vararg functions, a new parameter mode 'variadic'
has been added which causes a handler to be treated as variadic if
such a parameter with a mode is present.

As C requires any int type with rank less than int to be promoted
to int, and float to be promoted to double (when passed to a non-fixed
parameter in a variadic function), a 'promotedtype' and 'promote'
function have been added to foreign type descriptors to describe
the appropriate relation.

This patch adds varargs support to foreign C handlers, allowing
binding to a wider range of C functions.

The syntax for vararg handlers is similar to that of C:
```
  foreign handler myvarargs(in pFirst as Pointer, ...) returns CInt binds to ...
```
Using the token `...` to indicate the start of the variable
parameter list.

To implement vararg functions, a new parameter mode 'variadic'
has been added which causes a handler to be treated as variadic if
such a parameter with a mode is present.

As C requires any int type with rank less than int to be promoted
to int, and float to be promoted to double (when passed to a non-fixed
parameter in a variadic function), a 'promotedtype' and 'promote'
function have been added to foreign type descriptors to describe
the appropriate relation.
Comment thread libscript/src/script-execute.cpp Outdated
{
// Get the value in the register, this determines the type.
MCValueRef t_arg_value = nil;
t_arg_value = CheckedFetchRegister(p_arg_reg);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably just be MCValueRef t_arg_value = CheckedFetchRegister(p_arg_reg);


if (p_fields[i].mode == kMCHandlerTypeFieldModeVariadic)
{
p_field_count = i;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps it's worth asserting that p_field_count == i + 1 and i != 0 as these things are checked at (lc-)compile-time aren't they? Just to ensure we don't break the assumptions about the position of the variadic param internally anywhere.

@@ -202,10 +209,32 @@ class MCScriptForeignInvocation
break;

case kMCScriptForeignHandlerLanguageC:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably throw an error if a variadic foreign handler binds to java since it's not yet supported

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I fixed this in the MCJavaCheckSignature - it throws a binding error if there is a variadic parameter present.

This patch makes it so that JavaCheckSignature will fail if
an attempt is made to bind to a variadic signature.
This patch ensures that the position of the variadic parameter
is valid when creating a handler typeinfo. If it is not, the
creation fails, and an error is thrown.
This patch tidies up a case of variable definition and
initialization being in separate commands.
@runrevmark

Copy link
Copy Markdown
Contributor Author

@livecodeali Hopefully that's all the current feedback addressed appropriately.

@livecodeali

Copy link
Copy Markdown
Member

@livecode-vulcan review ok a8b2d34

@livecode-vulcan

Copy link
Copy Markdown
Contributor

💙 review by @livecodeali ok a8b2d34

@livecode-vulcan

Copy link
Copy Markdown
Contributor

😎 test success a8b2d34

  • try-community-armv6-android-api8: success
  • try-community-armv6-android-api9: success
  • try-community-js-emscripten-sdk1.35: success
  • try-community-universal-ios-iphoneos10.3: success
  • try-community-universal-ios-iphonesimulator10.3: success
  • try-community-universal-mac-macosx10.6: success
  • try-community-universal-mac-macosx10.9: success
  • try-community-x86-linux-debian7: success
  • try-community-x86-linux-debian8: success
  • try-community-x86_64-linux-debian7: success
  • try-community-x86_64-linux-debian8: success
  • try-community-x86-win32: success
  • try-community-x86_64-win32: success

@runrevmark runrevmark merged commit 1e8f1cf into livecode:develop Jul 7, 2017
@runrevmark runrevmark deleted the ffi-varargs branch July 7, 2017 09:46
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants