-
Notifications
You must be signed in to change notification settings - Fork 38
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
C API Get module from an instance #599
Conversation
Codecov Report
@@ Coverage Diff @@
## master #599 +/- ##
=======================================
Coverage 98.26% 98.26%
=======================================
Files 63 63
Lines 9282 9294 +12
=======================================
+ Hits 9121 9133 +12
Misses 161 161 |
fd57d9d
to
833a2b3
Compare
833a2b3
to
20e1775
Compare
/// | ||
/// @note The returned pointer respresents non-owning, "view"-access to the module and must not be | ||
/// passed to fizzy_free_module. | ||
const FizzyModule* fizzy_get_instance_module(FizzyInstance* instance); |
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.
const FizzyModule* fizzy_get_instance_module(FizzyInstance* instance); | |
const FizzyModule* fizzy_instance_get_module(FizzyInstance* instance); |
?
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 think this may be better wording, but we already have get_instance_memory
, so I suggest we go with what is the PR and then rename all of them at once if we want.
I guess this is only a "short term" workaround until we have C++ API to make all this work on the instance? |
I don't know, do you imagine that we would duplicate all module inspection functions for an instance? |
20e1775
to
5993c8b
Compare
Probably this conversation originates from where |
auto instance = fizzy_instantiate(module, nullptr, 0); | ||
EXPECT_NE(instance, nullptr); | ||
|
||
auto instance_module = fizzy_get_instance_module(instance); |
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.
How about adding EXPECT_EQ(module, instance_module)
?
Module is still consumed. |
Wasn't the whole point of the |
95246c5
to
9991225
Compare
No, for that it would need to be changed to The point was for |
5993c8b
to
022a155
Compare
022a155
to
f714cad
Compare
No description provided.