wit-bindgen icon indicating copy to clipboard operation
wit-bindgen copied to clipboard

Separate rust stubs

Open EugenDueck opened this issue 4 months ago • 2 comments

Changes

--stubs
    If true, generate stub implementations for any exported functions, interfaces, and/or resources

to

--stubs <STUBS>
    Whether to generate stub implementations for any exported functions, interfaces, and/or resources.
    
    Valid values are:
    
    - `omit`: Stubs will not be generated.
    
    - `embedded`: Stubs will be generated in the bindings file.
    
    - `separate`: Stubs will be generated in a separate _impl file.
    
    [default: omit]

resolves #1347

I put this in draft mode, because there is still things to discuss.

  1. The separate file is now called {world_name}_impl.rs.

However, I'd prefer the implementation to be in {world_name}.rs, and the bindings file in {world_name}_bindings.rs. So I suggest to output bindings into {world_name}_bindings.rs. Even when --stubs != separate, so that the names are consistent, allowing for this use case: First time around, you run wit-bindgen --stubs separate. Then you make manual edits to the implementation, and when the wit file changes, you just do a wit-bindgen, so as not to overwrite your manual edits. As that file name change is perhaps a change users won't agree to, I wanted to discuss this first, before making that change.

  1. codegen tests for --stubs separate

Currently, there only embedded stubs get codegen-tested. I have run all codegen cases for the --stubs separate case as well locally, but it's not a straightforward change to run tests for both embedded and separate, so I have for now put the latter case behind a false constant:

const STUB_SEPARATE: bool = false;

So it can at least be tested manually by flipping that const to true. But I think we should always be testing both, because putting stubs into a different file opens a couple of different pitfalls.

Please let me know what you think, what changes you'd like to see etc.

Eugen

EugenDueck avatar Aug 15 '25 03:08 EugenDueck

Thanks for working on this.

re 1, I don't see harm in making that change for all cases, but I don't use the cli (I tend to just use the proc macro on its own) so I'm not very opinionated. If someone else has an opinion here I'll defer to it.

re 2, Yes, both modes should be tested. We should somehow factor the test runner such that it can test more configs per LanguageMethods impl?

pchickey avatar Aug 15 '25 17:08 pchickey

We should somehow factor the test runner such that it can test more configs per LanguageMethods impl?

It was relatively easy to refactor, by removing the distinction between "test variants" and the test base case. Codegen cmd line args for the. base case were also forced upon the variants. Incidentally makes the code a little cleaner and more DRY.

EugenDueck avatar Aug 16 '25 04:08 EugenDueck