bytecodealliance
Remove sig data arg and ret fields to reduce size
Ooh, I missed an important detail! args and rets are both in the same abi_args array, so the first one doesn't necessarily start at 0.
More precisely, SigSet::abi_args is laid out with each signature having rets first and then args. This is ensured by from_func_sig, which has a comment saying "Handle retvals first, because we may need to add a return-area arg to the args."
As a result,
-
self.args_start==self.rets_end. -
self.rets_start==prev.args_end, or 0 if there is no previousSigData.
I believe that's the cause of the test failures. They were all off-by-one because the inputs that failed had exactly one return value.
I think this means we could save some more space by keeping one u16 length and one u32 offset in each SigData, but I don't think we should try to do that until we've fixed this version using two u32 offsets. Anyway, if I'm counting right, reducing a u32 to a u16 won't make the struct smaller right now because of padding and alignment, but it could help later combined with other changes.
I'm not totally happy with how confusing this will be for anyone trying to maintain this code in the future, but I think it's worth doing if we add some comments. In particular, extend the comment at the beginning of from_func_sig to mention that the args and rets methods rely on the order in which they're added to abi_args; and add a comment to the args and rets methods saying to see from_func_sig for details.
I believe this is correct and a good change, but it's tricky enough that I'd like to get a second review from either @cfallin or @fitzgen.