wasm-encoder: Return Indices from builder functions where applicable

[esoterra]

This refactor changes the returns of builder methods which create indexed entries (e.g. types) so that they return the index of the newly created entry. This means users don't need to otherwise calculate the value or know what it will be.

For example, the README no longer needs the line let type_index = 0; since it can be obtained when types.function(...) is called.

// Encode the type section.
let mut types = TypeSection::new();
let params = vec![ValType::I32, ValType::I32];
let results = vec![ValType::I32];
let type_index = types.function(params, results);
module.section(&types);

// Encode the function section.
let mut functions = FunctionSection::new();
functions.function(type_index);
module.section(&functions);

[peterhuene]

I don't think this change makes sense for the component model sections because there isn't a 1:1 with section and index space like there is in core modules.

If we pare this down to only the module type sections, then I think this would be fine.

[esoterra]

That makes sense, I've reverted all the component model indexing logic for now and we can revisit it at a future point.
@alexcrichton mentioned that for core wasm it's also sometimes incorrect since imports exist at the beginning of the index space for some things.

From the spec:

The index space for functions, tables, memories and globals includes respective imports declared in the same module. The indices of these imports precede the indices of other definitions in the same index space

Reviewing the index spaces, only a few make sense to include:

• typeidx
• funcidx (includes imports)
• tableidx (includes imports)
• memidx (includes imports)
• globalidx (includes imports)
• elemidx
• dataidx
• localidx (not created by builder)
• labelidx (not created by builder)

So, I'm going to further pare this down to types, elements, and data.
It's a little sad, because I think memory and table indexes being used in data and elements is one of the nicer use cases but this is all we can currently do soundly without a bigger change.

[peterhuene]

@alexcrichton is, as usual, correct! We'd have to move to a pattern similar to the nested encoders in component type encoding where the module being encoded hands out new section encoders so that the index spaces are tracked across all sections being encoded in the module.

That'd be a pretty big breaking change, although it sure would be nice if component encoding tools didn't have to manually track the index spaces as component index spaces are a bit more complicated than index spaces in a module.

Stepping back for a second, I do wonder if it's worth taking the pared-down change when the index of the added item can be retrieved from the section with the len method before the call to insert a section item already; granted, that's not as nice an interface of "here's the index of what you just added", but I do wonder if it's worth changing part of the encoding API for a relatively minor ergonomic benefit.

[alexcrichton]

I personally think it's ok to remove the -> &mut Self builder-style pattern since I don't think it's really used all that often with wasm-encoder anyway. In that sesnse I think returning -> u32 is fine, but while I don't think the FuncSection could return raw indices it could still have something like FuncSection::count(&self) -> u32 which returns the count of items added there. The documentation for the function could indicate that the count must be added to the number of imports to find the actual index.

For consistency though it might make the most sense to have len-style methods on most section builders rather than some returning indices and some returning &mut Self perhaps?