Skip to content

api: resolving imported globals #637

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

Merged
merged 4 commits into from
Jan 27, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 42 additions & 0 deletions lib/fizzy/instantiate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,33 @@ ExternalFunction find_imported_function(const std::string& module, const std::st
return {it->function, module_func_type};
}

ExternalGlobal find_imported_global(const std::string& module, const std::string& name,
GlobalType module_global_type, const std::vector<ImportedGlobal>& imported_globals)
{
const auto it = std::find_if(imported_globals.begin(), imported_globals.end(),
[module, name](const auto& func) { return module == func.module && name == func.name; });

if (it == imported_globals.end())
{
throw instantiate_error{"imported global " + module + "." + name + " is required"};
}

if (module_global_type.value_type != it->type)
{
throw instantiate_error{"global " + module + "." + name +
" value type doesn't match imported global in module"};
}
if (module_global_type.is_mutable != it->is_mutable)
{
throw instantiate_error{"global " + module + "." + name +
" mutability doesn't match imported global in module"};
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Should we check here also that it->value != nullptr ?
(note that it's checked in instantiate anyway)

Copy link
Member

Choose a reason for hiding this comment

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

I think it is fine as is. Perhaps a comment here could be useful to say that instantiate will validate the presence of a value.

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 a comment.

// instantiate function will validate whether `it->value` is not nullptr.

return {it->value, module_global_type};
}

std::optional<uint32_t> find_export(const Module& module, ExternalKind kind, std::string_view name)
{
const auto it = std::find_if(module.exportsec.begin(), module.exportsec.end(),
Expand Down Expand Up @@ -423,6 +450,21 @@ std::vector<ExternalFunction> resolve_imported_functions(
return external_functions;
}

std::vector<ExternalGlobal> resolve_imported_globals(
const Module& module, const std::vector<ImportedGlobal>& imported_globals)
{
std::vector<ExternalGlobal> external_globals;
for (const auto& import : module.importsec)
{
if (import.kind != ExternalKind::Global)
continue;

external_globals.emplace_back(
find_imported_global(import.module, import.name, import.desc.global, imported_globals));
}
return external_globals;
}

std::optional<FuncIdx> find_exported_function(const Module& module, std::string_view name)
{
return find_export(module, ExternalKind::Function, name);
Expand Down
16 changes: 16 additions & 0 deletions lib/fizzy/instantiate.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,22 @@ struct ImportedFunction
std::vector<ExternalFunction> resolve_imported_functions(
const Module& module, const std::vector<ImportedFunction>& imported_functions);

// Global that should be used by instantiate as import, identified by module and global name.
struct ImportedGlobal
{
std::string module;
std::string name;
Value* value = nullptr;
Copy link
Member

@axic axic Jan 27, 2021

Choose a reason for hiding this comment

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

Hmm, module/name/value does not seem to be used ye?

module/name is used in find_imported_global, but where is value used?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

find_imported_global puts it into returned ExternalGlobal

Copy link
Member

Choose a reason for hiding this comment

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

Ah yes I missed the return value.

ValType type = ValType::i32;
bool is_mutable = false;
};

// Create vector of ExternalGlobals ready to be passed to instantiate.
// imported_globals may be in any order, but must contain globals for all of the imported global
Copy link
Member

Choose a reason for hiding this comment

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

Just realised these comments are no doxygen, neither for resolve_imported_functions. If we want to doxygenize them, lets do it in a new PR.

// names defined in the module.
std::vector<ExternalGlobal> resolve_imported_globals(
const Module& module, const std::vector<ImportedGlobal>& imported_globals);

/// Find exported function index by name.
std::optional<FuncIdx> find_exported_function(const Module& module, std::string_view name);

Expand Down