-
Notifications
You must be signed in to change notification settings - Fork 134
Fix README.md host function calling examples to match current API #682
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There were a few problems with this:
The main example has a hardcoded value that we don't have a package for
hyperlight_host::GuestBinary::FilePath(hyperlight_testing::simple_guest_as_string().unwrap()),
the guest example had some unused functions:
use hyperlight_guest_bin::host_comm::{call_host_function, call_host_function_without_returning_result};
And a few types wrong:
error[E0308]: mismatched types
--> src/main.rs:41:9
|
37 | let print_output_def = GuestFunctionDefinition::new(
| ---------------------------- arguments to this function are incorrect
...
41 | print_output as i64,
| ^^^^^^^^^^^^^^^^^^^ expected `usize`, found `i64`
|
note: associated function defined here
--> /home/jstur/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/hyperlight-guest-bin-0.7.0/src/guest_function/definition.rs:40:12
|
40 | pub fn new(
| ^^^
help: you can convert an `i64` to a `usize` and panic if the converted value doesn't fit
|
41 | (print_output as i64).try_into().unwrap(),
| + +++++++++++++++++++++
In addition, we should give enough information in the readme to get started, but in this case its not clear that we also need some cargo/config.toml
that tells the guest to build a unknown target. should be similiar to what is in simpleguest.
otherwise we end up with an error:
cargo build
Compiling guest v0.1.0 (/home/jstur/projects/test-hl/guest)
error: unwinding panics are not supported without std
|
= help: using nightly cargo, use -Zbuild-std with panic="abort" to avoid unwinding
= note: since the core library is usually precompiled with panic="unwind", rebuilding your crate with panic="abort" may not be enough to fix the problem
error: could not compile `guest` (bin "guest") due to 1 previous error
Fixed all the issues you identified:
All examples now compile correctly and match the working patterns from the existing codebase. Commit: dfe87a7 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested these changes locally and it works
dfe87a7
to
667abe7
Compare
Co-authored-by: jsturtevant <[email protected]>
667abe7
to
8814dd9
Compare
The README.md examples for host function calling were outdated and didn't match the current API, causing confusion for users trying to follow the documentation.
Issues Fixed
1. Guest Function API Changes
The guest example was using the old two-step API pattern:
Before (outdated):
After (current API):
2. Import Corrections
Fixed incorrect import statements:
get_flatbuffer_result_from_int
→get_flatbuffer_result
3. Host Example Cleanup
Removed unused imports that were causing confusion:
Arc
,Mutex
,ParameterValue
,ReturnType
,HostFunction0
4. Function Call Fixes
Fixed generic type annotation placement and error handling in the host example:
<i32>
to the correct position oncall_guest_function_by_name
Verification
src/hyperlight_host/examples/
The documentation now accurately reflects the current host function calling API introduced in the changes mentioned in the issue.
Fixes #530.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.