Avoid creating temporary vector copies when checking signature

[mjbshaw]

I was doing some local bench marking and found a significant amount of wasm-validate's time came from copying (and then destroying) temporary vectors here. Currently CheckSignature() is calling PrintStackIfFailed() and passing sig (which is already a `TypeVector), whic creates a copy. This change eliminates the unnecessary vector copy and adds a static assertion that should prevent this from happening again.

[sbc100]

Under what circumstances do these args end up being vectord? Multi-value?

[mjbshaw]

I linked to where it happens in the comment. CheckSignature() is passing its sig vector to PrintStackIfFailed(). I don't see any other occurrances, though, so an easy alternative would be to change line 257 to use PrintStackIfFailedV() instead.

[sbc100]

I linked to where it happens in the comment. CheckSignature() is passing its sig vector to PrintStackIfFailed(). I don't see any other occurrances, though, so an easy alternative would be to change line 257 to use PrintStackIfFailedV() instead.

Hmm.. yes it does looks like PrintStackIfFailedV would be the correct thing to call here. Otherwise are we not making a vector of a vector? It seems like PrintStackIfFailed should be called only with individual types as arguments.

[mjbshaw]

yes it does looks like PrintStackIfFailedV would be the correct thing to call here.

Okay, I've updated the PR to instead directly call PrintStackIfFailedV.

Otherwise are we not making a vector of a vector?

There's only a single argument in the variadic template, so {args...} desugars to {arg}, where arg is a vector derived from the sig argument. This form of braced initialization in this situation does not try to create a vector-of-vectors.

It seems like PrintStackIfFailed should be called only with individual types as arguments.

Yeah that seems to be the intention.

[mjbshaw]

Can you update the PR description and title?

Done.

I wonder if we could add some type checking to PrintStackIfFailed such that it verifies that each its arguments is indeed a Type? (and not come more complex type like this)?

I've added a static_assert that enforces this (and confirmed it works as intended).