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

Improve importing things #130

Merged
merged 17 commits into from
Jan 29, 2019
Merged

Improve importing things #130

merged 17 commits into from
Jan 29, 2019

Conversation

lachlansneff
Copy link
Contributor

@lachlansneff lachlansneff commented Jan 29, 2019

Now, you can import memories, tables, and globals like such:

let memory = Memory::new(MemoryDesc {
    min: 1,
    max: Some(1),
    shared: false,
})?;

let table = Table::new(TableDesc {
    ty: ElementType::Anyfunc,
    min: 10,
    max: None,
})?;

let global = Global::new(Value::I32(42));

let import_object = imports! {
    "env" => {
        "print_i32" => func!(print_num, [i32] -> [i32]),
        "memory" => memory,
        "global" => global,
        "table" => table,
    },
};

@syrusakbary
Copy link
Member

Love this!

let local_memory = *ctx.memories.add(memory_index.index());
let memory = (*local_memory).memory as *mut DynamicMemory;

if let Some(old) = (*memory).grow(delta, &mut *local_memory) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a way to make this less verbose? The js API for this is memory.grow(1)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The user can just call memory.grow(delta). This cruft in this function is because it's specifically accessing a particular type of memory. The user-exposed api doesn't expose that.

lib/runtime-core/src/vm.rs Outdated Show resolved Hide resolved
@@ -20,6 +22,8 @@ pub struct ModuleEnv<'module, 'isa> {
pub signatures: Map<SigIndex, ir::Signature>,
globals: Map<GlobalIndex, cranelift_wasm::Global>,
func_bodies: Map<LocalFuncIndex, ir::Function>,
pub deduplicated: PrimaryMap<cranelift_wasm::SignatureIndex, SigIndex>,
duplicated: HashMap<SigIndex, cranelift_wasm::SignatureIndex>,
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe the naming could be more specific deduplicated_signatures duplicated_signatures or signatures_all/signatures_unique

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This could probably be designed better tbh. I was just trying to get it to work as quickly as possible.

@syrusakbary syrusakbary merged commit c183a11 into master Jan 29, 2019
@lachlansneff lachlansneff deleted the feature/improve-importing branch January 29, 2019 23:56
MarkMcCaskey pushed a commit that referenced this pull request Apr 14, 2020
This commit fixes a bug where the lock guard (for concurrently accessing
the same scope from different threads) had basically no effect, due to
being bound to a temporary only.
nlewycky pushed a commit that referenced this pull request Aug 13, 2020
fix(compiler-llvm) Remove an unused import
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.

3 participants