Added context trampoline into runtime #481
Conversation
|
bors try |
lib/runtime-c-api/src/trampoline.rs
Outdated
| ctx: *const c_void, | ||
| ) -> usize { | ||
| let b = &mut *(b as *mut TrampolineBufferBuilder); | ||
| b.add_function(f as *const CallTarget, ctx as *const CallContext) |
There was a problem hiding this comment.
Please, check that ctx isn't a null pointer. Use update_last_error to raise an error accordingly.
There was a problem hiding this comment.
Also, check that f and b aren't null.
There was a problem hiding this comment.
Do we really need to check null pointers explicitly that way?
It's easy to debug SIGSEGV's caused by null pointers.
There was a problem hiding this comment.
This is what we did in the other function, it's debatable :-).
| /// and passes the array into `target` as the second argument when called. The first argument | ||
| /// of `target` is the `context` specified here. | ||
| /// | ||
| /// Note that non-integer parameters/variadic functions are not supported. |
There was a problem hiding this comment.
Does it support floats (I think it does not matter since the bits can be interpreted as a float, but I prefer to ask)?
There was a problem hiding this comment.
Floats are passed in different registers and it would be very complex to handle this case. I think it's better to just support integer parameters.
There was a problem hiding this comment.
So trampoline doesn't work with floats. It's important to know!
|
bors r+ |
481: Added context trampoline into runtime r=losfair a=syrusakbary Based on #450 Co-authored-by: losfair <[email protected]>
Based on #450