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

capi: Remove dynamic allocation in find_exported_function #791

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

gumb0
Copy link
Collaborator

@gumb0 gumb0 commented May 5, 2021

This is a PoC to get rid of extra dynamic allocation in C API for imported/exported functions at the expense of

  • some changes/complications in API (members added to FizzyExternalFunction, but the upside is that fizzy_free_exported_function is removed)
  • not being able to access C++ host functions, only C host functions available on C side.
  • some C API details leaking into C++ side.

@codecov
Copy link

codecov bot commented May 5, 2021

Codecov Report

Merging #791 (fd6ade7) into master (2ea01bc) will decrease coverage by 0.06%.
The diff coverage is 85.71%.

@@            Coverage Diff             @@
##           master     #791      +/-   ##
==========================================
- Coverage   99.23%   99.16%   -0.07%     
==========================================
  Files          79       79              
  Lines       12394    12396       +2     
==========================================
- Hits        12299    12293       -6     
- Misses         95      103       +8     
Flag Coverage Δ
rust 99.90% <ø> (ø)
spectests 90.26% <0.00%> (-0.29%) ⬇️
unittests 99.09% <85.71%> (-0.08%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
lib/fizzy/capi.cpp 95.17% <66.66%> (-2.39%) ⬇️
lib/fizzy/instantiate.hpp 96.55% <85.71%> (-3.45%) ⬇️
lib/fizzy/instantiate.cpp 100.00% <100.00%> (ø)
test/unittests/capi_test.cpp 99.89% <100.00%> (+<0.01%) ⬆️
test/utils/fizzy_c_engine.cpp 96.66% <100.00%> (ø)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant