Added function to merge ImportObjects

[tprk77]

I thought I would try implementing the ImportObject merging I needed. The main use case is merging the Emscripten imports from generate_emscripten_env with the usual ones, i.e., print_str from the wasmer-rust-example.

Here's where I'm using it, in my attempt to update the wasmer-rust-example to run Emscripten stuff:

https://github.com/tprk77/wasmer-rust-example/blob/emscripten-wip/src/main.rs

Apologies in advance if my code is wacky, I'm still kind of learning Rust. I'd appreciate any feedback you might have.

[xmclark]

Don't be too hard on yourself. Many of us are still learning rust 😄.

This code looks great! Would you be willing to write some unit tests?

[xmclark]

I think merge may be a better name.

[tprk77]

Sounds good to me.

[tprk77]

Yeah, I can write some tests, I just haven't done that before in Rust. Are the tests in emscripten/src/utils.rs a good example of how to structure those?

[xmclark]

@tprk77 We do not have any conventions for this kind of testing. You can't do anything wrong by making tests. If we feel like reorganizing, we will, but the win with tests outweighs any refactoring costs in the future.

For now, putting the tests in the same file is fine. Cargo will find the tests no matter what when we run cargo test. To add some tests, append some code like this to the end of the file:

#[cfg(test)]
mod tests {
    use super::*;
    #[test]
    fn test_merge imports() {
        // assert!(truthy_thing, "message if fails")
    }
}

[lachlansneff]

Can you also add some documentation here about the behavior of this method during edge cases (e.g. Two items with the same name)?

[tprk77]

Ok, I think I've got everything in order. Let me know if I need to add or change anything else.

[syrusakbary]

@tprk77 we are thinking on adding an extend method to the ImportObject so it's much easier to do it via Extend, FromIterator and IntoIterator traits.

[tprk77]

@syrusakbary Sounds like a good idea. I wouldn't mind working on it.

I have a question about the details. I have mostly been thinking of Namespace as a set of exports, and ImportObject as a set of namespaces. So for a namespace, extend would add its exports to the set which makes sense. But for ImportObject what should extend actually do?

If it's analogous to a set, it should just add a namespace to the set. I think using extend for merge semantics on the underlying namespaces may be a bit confusing to the caller. But on the other hand, it's probably what people usually want to happen? Do you have an opinion on this either way?

[tprk77]

I implemented iterators for import objects and like-namespaces. I wrote tests for both.

Implementing the iterator for LikeNamespace was pretty tricky because of it being a trait-object. The iterator is boxed so different implementations can return different iterators.

I got rid of the get_all_exports, and replaced it with get_export_names and ExportIter. I thought you would all like that, it seems much cleaner to me.

I kept the merge function mostly the same for now, since I'm not sure what you want to do with that.

Please send your comments my way, I'm still happy to work on this.

[tprk77]

This PR doesn't work anymore.