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: Export inspection functions #686

Merged
merged 2 commits into from
Jan 26, 2021
Merged

capi: Export inspection functions #686

merged 2 commits into from
Jan 26, 2021

Conversation

gumb0
Copy link
Collaborator

@gumb0 gumb0 commented Jan 7, 2021

No description provided.

@codecov
Copy link

codecov bot commented Jan 7, 2021

Codecov Report

Merging #686 (a83cf57) into master (43dcc2b) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #686   +/-   ##
=======================================
  Coverage   99.30%   99.31%           
=======================================
  Files          72       72           
  Lines       10420    10470   +50     
=======================================
+ Hits        10348    10398   +50     
  Misses         72       72           
Flag Coverage Δ
spectests 91.50% <ø> (ø)
unittests 99.31% <100.00%> (+<0.01%) ⬆️

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

Impacted Files Coverage Δ
lib/fizzy/capi.cpp 98.87% <100.00%> (+0.02%) ⬆️
test/unittests/capi_test.cpp 100.00% <100.00%> (ø)

@gumb0 gumb0 force-pushed the capi-exports-inspection branch 3 times, most recently from 0d84006 to 2db892b Compare January 12, 2021 15:41
@gumb0 gumb0 marked this pull request as ready for review January 12, 2021 15:43
@gumb0 gumb0 requested review from chfast and axic January 12, 2021 16:12
} FizzyExternalKind;

/// Export description.
typedef struct FizzyExportDescription
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same comment as with FizzyImportDescription: #683 (comment). Please add a remark about the lifetime of name.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added.

@@ -133,6 +133,27 @@ typedef struct FizzyExternalGlobal
FizzyGlobalType type;
} FizzyExternalGlobal;

/// External kind.
typedef enum FizzyExternalKind
Copy link
Member

@axic axic Jan 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, this is in conflict with #683, so need to merge one first then rebase.

/// @return Number of exports in the module.
uint32_t fizzy_get_export_count(const FizzyModule* module);

/// Get the import description defined in the module.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Import?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

Copy link
Member

@axic axic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good too, just some of those documentation questions.

include/fizzy/fizzy.h Outdated Show resolved Hide resolved
/// Export kind.
FizzyExternalKind kind;
/// Index of exported function or table or memory or global.
/// #kind determines what is this index of.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// #kind determines what is this index of.
/// #kind determines what is this the index of.

@axic axic merged commit 4e4d86f into master Jan 26, 2021
@axic axic deleted the capi-exports-inspection branch January 26, 2021 22:55
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.

2 participants