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

Improve callback function support #364

Open
vchuravy opened this issue Dec 30, 2021 · 2 comments
Open

Improve callback function support #364

vchuravy opened this issue Dec 30, 2021 · 2 comments
Labels
good first issue Good for newcomers help wanted Extra attention is needed

Comments

@vchuravy
Copy link
Collaborator

Right now we are generating:

# typedef void ( * pmix_event_notification_cbfunc_fn_t ) ( pmix_status_t status , pmix_info_t * results , size_t nresults , pmix_op_cbfunc_t cbfunc , void * thiscbdata , void * notification_cbdata )
const pmix_event_notification_cbfunc_fn_t = Ptr{Cvoid}

And I find myself writing:

function notification_fn(evhdlr_registration_id::Csize_t, status::PMIx.API.pmix_status_t, source::Ptr{PMIx.API.pmix_proc_t},
                         info::Ptr{PMIx.API.pmix_info_t}, ninfo::Csize_t,
                         results::Ptr{PMIx.API.pmix_info_t}, nresults::Csize_t,
                         cbfunc::PMIx.API.pmix_event_notification_cbfunc_fn_t, cbdata::Ptr{Cvoid})::Nothing
...
end

callback = @cfunction(notification_fn, Cvoid, (Csize_t, PMIx.API.pmix_status_t, Ptr{PMIx.API.pmix_proc_t}, Ptr{PMIx.API.pmix_info_t}, Csize_t, Ptr{PMIx.API.pmix_info_t}, Csize_t, Ptr{PMIx.API.pmix_event_notification_cbfunc_fn_t}, Ptr{Cvoid}))

The type annotation in notation_fn is not necessary, but I do find it helps with these long signature.

I was thinking of defining a custom struct, but that requires @cfunction($) and that is not fully supported everywhere.
So maybe a macro that makes the cfunction easier?

@melonedo
Copy link
Contributor

Since the function signature that @cfunction needs is very tedious to write, I think we can export the Cvoid, (Csize_t, PMIx.API.pmix_status_t, Ptr{PMIx.API.pmix_proc_t}, Ptr{PMIx.API.pmix_info_t}, Csize_t, Ptr{PMIx.API.pmix_info_t}, Csize_t, Ptr{PMIx.API.pmix_event_notification_cbfunc_fn_t}, Ptr{Cvoid}) part as some constant pmix_event_notification_cbfunc_fn_t_signature, then we can write

callback = @cfunction(notification_fn, $(pmix_event_notification_cbfunc_fn_t_signature...))

But this won't help you write the callback function itself.

@Gnimuc
Copy link
Member

Gnimuc commented Dec 30, 2021

I'm thinking of doing something like this:

julia> function foo_callback(x::Csize_t)::Cint
           __impl_foo_callback(x)
       end
foo_callback (generic function with 1 method)

julia> cb = @cfunction(foo_callback, Cint, (Csize_t,))  # this should be generated in `__init__()`
Ptr{Nothing} @0x0000000111e59180

julia> ccall(cb, Cint, (Csize_t,), 0)
ERROR: UndefVarError: __impl_foo_callback not defined
Stacktrace:
 [1] foo_callback(x::UInt64)
   @ Main ./REPL[1]:2
 [2] top-level scope
   @ ./REPL[3]:1

julia> __impl_foo_callback(x) = x + 1
__impl_foo_callback (generic function with 1 method)

julia> ccall(cb, Cint, (Csize_t,), 0)
1

In this way, users can define __impl_foo_callback without annotating argument types.

@Gnimuc Gnimuc added good first issue Good for newcomers help wanted Extra attention is needed labels Jan 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants