Simplify bindings implementation

[syrusakbary]

After taking a quick look on the project, I realized there was a bit of shared logic between the binding generators that could be simplified.
Eg: https://github.com/bytecodealliance/witx-bindgen/blob/main/crates/gen-wasmtime/src/lib.rs / https://github.com/bytecodealliance/witx-bindgen/blob/main/crates/gen-wasmtime-py/src/lib.rs

As more bindings get implemented this might become more challenging to maintain. I was wondering if there can be an implementation based on the visitor pattern.

Visitor pattern

GraphQL designed a very simple pattern (visitor pattern) to walk through the AST in a simple an succinct way.
Some overview: https://www.graphql-code-generator.com/docs/custom-codegen/using-visitor
Reference implementation in Python: https://github.com/graphql-python/graphql-core/blob/main/src/graphql/language/visitor.py

Example of schema printer based on the Visitor pattern: https://github.com/graphql-python/graphql-core/blob/main/src/graphql/utilities/print_schema.py

Candid's approach

Candid (Dfinity's version of Interface Types), also follows a similar pattern where it's bindings are simple and concise to define:
https://github.com/dfinity/candid/blob/master/rust/candid/src/bindings/typescript.rs

I was wondering if it could make sense to refactor some of the codebase to allow easy maintainability of the generators while repeating as minimum as possible in the bindings generator code.

Thoughts?

[alexcrichton]

Thanks for the report, but I'm not sure that this would really simplify the bindings all that much. Not to say I've written the simplest possible bindings as-is right now, I'm sure they could be improved! In any case I'm happy to review PRs to update the implementations (a prototype in one generator should be fine).

[syrusakbary]

Looking a bit more into it, I think it might be useful to add another layer before the binding generation that processes the data to be easily consumed by the generator.

For example, right now this code:

record person {
  name: string,
  age: u32,
}
hello: function(who: person) -> string
hello2: function(who: person) -> string

Generates the following in Javascript:

  imports["input"]["hello"] = function(arg0, arg1, arg2, arg3) {
    const memory = get_export("memory");
    const realloc = get_export("canonical_abi_realloc");
    const ptr0 = arg0;
    const len0 = arg1;
    const ret = obj.hello({
      name: UTF8_DECODER.decode(new Uint8Array(memory.buffer, ptr0, len0)),
      age: arg2 >>> 0,
    });
    const ptr1 = utf8_encode(ret, realloc, memory);
    const len1 = UTF8_ENCODED_LEN;
    data_view(memory).setInt32(arg3 + 8, len1, true);
    data_view(memory).setInt32(arg3 + 0, ptr1, true);
  };
  imports["input"]["hello2"] = function(arg0, arg1, arg2, arg3) {
    const memory = get_export("memory");
    const realloc = get_export("canonical_abi_realloc");
    const ptr0 = arg0;
    const len0 = arg1;
    const ret = obj.hello2({
      name: UTF8_DECODER.decode(new Uint8Array(memory.buffer, ptr0, len0)),
      age: arg2 >>> 0,
    });
    const ptr1 = utf8_encode(ret, realloc, memory);
    const len1 = UTF8_ENCODED_LEN;
    data_view(memory).setInt32(arg3 + 8, len1, true);
    data_view(memory).setInt32(arg3 + 0, ptr1, true);
  };

As we can see, the hello and hello2 implementations are the same (obviating the obj. call). I think it might be useful to add a layer that preprocesses the AST, such as grouping decoders by type (for example, person decoder, ...) before binding generation.

There might be other useful steps, such as deciding which imports are going to be required and precomputed for us (haven't got that far on the code yet, I'm sure there will also be likely more wins).
If you are open to it I can add a simple demo with a new binding and we can iterate on it before applying the technique globally?

[alexcrichton]

I'm hesitant to apply too many optimizations or such to the bindings just yet because interface types is still in flux and it makes it that much harder to update the bindings when interface types or the canonical abi changes. I'd also prefer to save performance/code size optimizations of the more advanced kind until they're more motivated. So far AFAIK witx-bindgen hasn't seen a lot of use so it's unclear to me what the pain points are and what needs to be optimized more.

For a new binding I'm also hesitant to add more at this time. I'm already not super happy with the number of generators in this repo itself as-is right now. I have added a bunch here for convenience and demo's sake, but I suspect that this will not be the final resting place for at least a few of these generators. I'd prefer to stem the tide sooner rather than later (after reaching a threshold of "this is proven enough with an array of bindings").

[alexcrichton]

I'm going to close this since I think it's in an ok state for now, but if specific issues arise in the future I think it'd be good to open targeted issues to track improvements.