Move evmc_host_interface out of evmc_host_context #427
Conversation
chfast
force-pushed
the
host_interface
branch
from
3e3f168
to
fa6180b
Compare
axic
force-pushed
the
host_interface
branch
from
fa6180b
to
c15d2dc
Compare
|
@axic Do you want me to finish Rust part here? |
chfast
force-pushed
the
host_interface
branch
from
3f59f1b
to
32b0982
Compare
chfast
force-pushed
the
host_interface
branch
3 times, most recently
from
076cb83
to
72da957
Compare
|
@chfast fixed compilation, but haven't reviewed the changes yet. |
axic
force-pushed
the
host_interface
branch
4 times, most recently
from
7dab19e
to
3f865ba
Compare
| @@ -132,7 +132,7 @@ mod tests { | |||
| get_block_hash: None, | |||
| emit_log: None, | |||
| }; | |||
| let mut host_context = ::evmc_sys::evmc_host_context { _unused: [] }; | |||
| let mut host_context = ::evmc_sys::evmc_host_context::default(); | |||
There was a problem hiding this comment.
Is it possible to pass a null pointer instead of creating host_context object?
There was a problem hiding this comment.
Yeah, that should be an option too. Will make Rust cry though, a mutable nullpointer reference lol.
There was a problem hiding this comment.
Just tried it and it feels like to be simple leaving it as is, for now.
There was a problem hiding this comment.
I tried it too, but feels like null pointers is Rust's advanced feature.
|
Ok, thanks a lot. I will clean up git history and put it for review. |
chfast
force-pushed
the
host_interface
branch
from
3f865ba
to
762a240
Compare
| @@ -345,7 +346,7 @@ fn build_execute_fn(names: &VMNameSet) -> proc_macro2::TokenStream { | |||
| use evmc_vm::EvmcVm; | |||
|
|
|||
| // TODO: context is optional in case of the "precompiles" capability | |||
| if instance.is_null() || context.is_null() || msg.is_null() || (code.is_null() && code_size != 0) { | |||
| if instance.is_null() || host.is_null() || context.is_null() || msg.is_null() || (code.is_null() && code_size != 0) { | |||
There was a problem hiding this comment.
Depending on #427 (comment) we may need to remove context.is_null() here.
There was a problem hiding this comment.
The context may always be null. VM never needs to dereference it.
Now the TODO comment should be applied to the host as in the case of "precompiles" it is not useful and may be null — this can be discussed further in #350.
There was a problem hiding this comment.
Fixed the context part. Lets leave host for #350.
chfast
force-pushed
the
host_interface
branch
from
500712f
to
71bdfad
Compare
|
@chfast simplified it to deal with null pointers. Also allows |
chfast
force-pushed
the
host_interface
branch
from
0339d84
to
f2afa8a
Compare
chfast
force-pushed
the
host_interface
branch
from
f2afa8a
to
4645d97
Compare
On the C API level (and also ABI) the
host_interface(think: an object vtable) andhost_context(think: pointer to an object's data) are separated. This actually makes creating language bindings easier - no need for workarounds as in https://github.com/ethereum/evmc/blob/master/bindings/go/evmc/evmc.go#L26-L30.Language bindings can still hide this by providing single OOP interface for Host (both for Client and VM side).