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

Askama macros for ffi function calls/decls should take a ffi_func. #232

Open
jhugman opened this issue Aug 11, 2020 · 2 comments
Open

Askama macros for ffi function calls/decls should take a ffi_func. #232

jhugman opened this issue Aug 11, 2020 · 2 comments

Comments

@jhugman
Copy link
Contributor

jhugman commented Aug 11, 2020

Currently, we have multiple macros with ffi_func in the name, but which take a func.

e.g.

{%- macro to_ffi_call_with_prefix(prefix, func) %}
{%- macro _arg_list_ffi_call(func) %}

This makes it much harder to generate internal function calls like object_free and string_alloc.

Ideally, macros should take more specific arguments:

{%- macro to_ffi_call_with_prefix(prefix, ffi_func) %}
{%- macro _arg_list_ffi_call(args) %}

Minimally steps:

  1. implement a method_arguments() method on FFIFunction which removes the first argument.
  2. copy throws() from Function/Method to FFIFunction, so the error code can be generated in the macro.
  3. rewrite the macros to stop using has_out_err
  4. rewrite to_ffi_call & to_ffi_call_with_prefix to call the ffi_func
  5. rename to_ffi_call_with_prefix to to_ffi_call_with_handle
  6. Make object_free, string_alloc, buffer_alloc etc use the newly useful macros.

┆Issue is synchronized with this Jira Task
┆Issue Number: UNIFFI-15

@rfk
Copy link
Collaborator

rfk commented Aug 13, 2020

Broadly +1 to this FWIW 👍

copy throws() from Function/Method to FFIFunction, so the error code can be generated in the macro.

Conceptually, I'm not sure about this detail. Once we have panic-safety, IIUC every FFIFunction will take an out &err parameter and can implicitly return an error code. The process of turning that error code into an error object of the appropriate type, seems analogous to the process of lifting a return value from its FFIType into the appropriate high-level Type. I think that argues for the conversion to be the responsibility of the enclosing Function or Method, rather than the lower-level FFIFunction. Having the FFIFunction know about the higher-level error type seems like a conceptual layering violation.

Put another way: FFIFunctions can return error codes, while Functions and Methods can throw error objects.

I'm not sure how to map that into a nice set of macros though. Maybe we could have a couple of different levels, e.g:

  • A low-level macro for emitting the call to the FFIFunction, which fills in the out &err parameter.
  • A mid-level macro that takes an Error type and an FFIFunction, calls the above, and handles lifting the err into a concrete foreign-language error object.
  • A high-level macro that takes a Function or Method and destructures it into a call to the above.

@jhugman
Copy link
Contributor Author

jhugman commented Aug 18, 2020

Put another way: FFIFunctions can return error codes, while Functions and Methods can throw error objects.

These internal method & function calls are being called from public APIs, so we probably should be enabling them to throw error objects. I agree with you about layering violations.

I'm not sure how to map that into a nice set of macros though.

Perhaps the best thing to do is: wait for the shake out from #221 landing and how that cleans up the macros, and then re-assess.

Currently there are only a handful of internal functions being called. It might just be cheaper to keep them in sync with the macros, rather than re-work the macros to account for these corners.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants