Feature/glue code generation

[tchataigner]

What It Does 🔎

This PR is a starting point of binding generation for holium transformations developed in Rust. It exposes a procedural macro that will wrap functions with necessary code to run in an Holium runtime.

Documentation Updates 📘

Pull Request

How To Test ✔️

cargo test

Linked Issues 🎫

• https://polyphene.myjetbrains.com/youtrack/issue/HTB-27

Related Pull Requests 🔀

Proposition of new issues

• https://polyphene.myjetbrains.com/youtrack/issue/HTB-112
• https://polyphene.myjetbrains.com/youtrack/issue/HTB-111

[tchataigner]

As of now it works with the development started in the repository holium on the branch feature/ee-interface-get

[PhilippeMts]

Derequesting my review, as discussed.

[PhilippeMts]

Thank you Thomas very much for this important PR.

You will find, on the files, small scattered comments I posted on the fly.

But what follows here are my most important comments.

Rationale for key_tree::Node and the GenerateNode trait

I'm sorry I don't get the rationale for key_tree::Node and the GenerateNode trait.

To facilitate communication between us, and as it may be useful for documentation, I drafted the following sequence diagram to summarize the mode of operation I expected.

Sources of the diagram are available as HTML comments in this comment.

If we call HoliumCBOR the subset of CBOR (without tags, maps,…) that we use to format data without keys in the Holium Framework, do we agree on the fact that the transformation wrapper, ie code generated by the procedural macro, receives such an HoliumCBOR object from the Holium client and is responsible for deserializing it into the only Rust-type input objects that the transformation developer knows of (and the other way around for outputs of the execution) ?

If we agree on that, I guess what is really needed is to integrate HoliumCBOR as a proper serde data format which would, without further considerations, allow any user-defined structure (in the broadest sense) to be directly usable as I/O transformation parameter.

Follow-up question : should we ask the user (ie the transformation developer) to explicitly import and use the derive macro ?

Indeed, the developer will use a procedural macro to target the transformations he may want to use in the Holium Framework (and this also could be subject of further discussions). But once a transformation is targeted, we may automatically know which structure should be derived into the serde framework.

In term of security, I don't see a reason to ask for explicit import. In term of DX, there are reasons not to, obviously. But in fact I personally don't understand the rationale in having to explicitly add the derive macro in any serde use case… and there must be one…

I would thus advise to keep it a manual process, and re-open this question when we work on the auto-import of non-holium-taylored libraries.

Generation of a WASM binary

In the shared sequence diagram, I share the idea that the output of the compilation is made of two parts : the pure transformation (almost the direct result of compilation of the developed Rust transformation), and a wrapper (build at compilation time with the procedural macro).

Do we agree on the fact that the SDK should help wrap transformations into a binary WASM build (not a library) that would then be run in quite a standard way in any WASM runtime? (Just as an example, we can see that wasmer seems to be ready to function that way and to return some kind of boxed value.)

If we agree on that, we should expect to have a big switch in the wrapper that allows to choose the right transformation in the module (a switch I cannot find in current implementation).

Other general comments

Documentation formating

You should use //! instead of /******/ in module headers to properly include them into the documentation.

Questionable ./tests/ directory

• We should implement integration tests, or remove this directory.
• I would not recommend using a new crate for integration tests, or justify it.
• I think ./tests/integration/src/ should lie in ./tests/src/.

Mixed successful and failing tests

• In proc-macro-tests, we should separate successful and failing test suites for clarity, using t.pass on successful ones and t.compile_fail only on failing ones.
• We may remove or at least explain update-references.sh and update-all-references.sh.

rustwasm/wasm-bindgen dependency and licensing

In current state of the code, it seems unlikely that we may respect the rustwasm/wasm-bindgen license without handling it properly.

The important question is, I think : how deeply do we rely on the code from rustwasm/wasm-bindgen ?

• if we share a lot of needs, then we would have to add it as a proper licensed dependency. But does it match our own plans on licensing ?
• If we want to consider we use this code as inspiration only, then we may be able to use it with no reference to their license, but would have to keep the amount of shared source code as minimal as possible. And I don't know if that sounds possible.

Do you have a take on that last point ?

[PhilippeMts]

On which rust side ?

[tchataigner]

Name inside the rust source code, updating description.

[PhilippeMts]

Maybe let's not get into the details of a no_mangling option right now, as we have other priorities.

[tchataigner]

Well it actually removes it as if for any reason the name of the function is a name that we will generate in our SDK then they will colide because #[no_mangle] exports publicly the function by its name.

[PhilippeMts]

Hasn't it already be handled in the macro_parse implementation for syn::Item ?

[tchataigner]

In macro_parse the code allows us to prepare our codegenby generating custom AST structs from and copying the current tokens in a new stream. Thencodegen` takes the new stream and generates code based on the generated AST structs.

So the to_tokens in macro_parse is only a matter of copying values while to_tokens in codegen is responsible for code generation.

[PhilippeMts]

Can you explain ?

[tchataigner]

Yep, if compiling to a wasm32 target then the name of the transformation to call from the runtime will be #exported_name. Otherwise it will be #holium_func_name. Intentions were to have customizable names for transformations at some point.

[PhilippeMts]

do we need extern "C" ?

[tchataigner]

Yes as it is a function exposed by our wasm module. As of now exposed Wasm has to be annotated with extern "C" as there is no other specific way