Strings: Add some interpreter support

[kripken]

This adds just enough support to be able to --fuzz-exec a small but realistic fuzz
testcase from Java.

To that end, just implement the minimal ops we need, which are all related to
JS-style strings. Full Strings support does not make sense to spend time on atm,
but this small PR unblocks fuzzing so it seems worth it.

[tlively]

I would prefer that we not change this method (and that we eventually remove it entirely). Code is written assuming that isData has a particular meaning, and if that meaning changes over time, those assumptions can be broken. It is much better for clients of the type API to be precise about what kinds of types they are querying.

[kripken]

This is a form of data, though, isn't it? It's a reference to data (with a notion of how to read the raw data as well).

Maybe I'm not seeing what you are proposing, though: What did you have in mind?

Note that this one-line change would become a many-line change if we need to find the many places that currently have isData() and turn them into isData() || isStringView() - I started down that path and quickly decided to change course. But maybe there is another option?

With that said, I do see your point that it's better when code locations have a precise meaning to what they use. But I don't think this changes that. I see 'isData' is meaning "is a reference to data"; concretely, all isData things are implemented by using the data field in Literal, so this is not arbitrary.

[tlively]

I'm worried about callers assuming that isData() is a convenient shorthand for isStruct() || isArray(), but lgtm for now if the fuzzer is happy. I still think it would be good to eventually do the larger NFC refactoring to eliminate isData() entirely to mitigate that kind of risk. Do you think that is a reasonable thing to do as a future cleanup?

[tlively]

Alternatively, we could find a more descriptive name for isData that gives it clear semantics beyond isStruct() || isArray(). Historically, isData had clear semantics because it corresponded to subtypes of heap type data, but when we removed data we did not remove isData().

isHeapAllocated might be a good candidate, if somewhat verbose.

[kripken]

I do feel there is a shared concept here, though? Again, it is all the types that use Literal::gcData, all the references to data. It is useful in our codebase to have a concept of all those things, because we need to test on them in all the places that use Literal::gcData. Otherwise each of those places would have "is struct or is array or is string or is stringView" which seems worse.

Is it the name data that meant "struct or array" in the spec that feels wrong to you?

[kripken]

(Our last comments raced, sorry mine isn't in response to yours.)

isHeapAllocated could be shortened to isHeapType 😆 But yeah, this is kind of "data that is heap allocated", but in the stronger sense of our internals. i31 is a heap type but does not store itself using Literal::gcData, so it isn't "heap allocated using Literal::gcData"... I'm not sure what the best name is here, but ignoring the history of the term, we have Literal::gcData now so something with data or gcData seems right?

[tlively]

i31 is never heap allocated (or at least, it shouldn't be), so it makes sense that it would be excluded from isHeapAllocated. It's much less clear that it should be excluded from isData!

isHeapData could work as well and is slightly shorter than isHeapAllocated.

I would prefer that we not name something in the wasm-type.h API based on internal details of the Literal API, though.

[kripken]

I see. Ok, let's keep thinking about this, I sort of understand where you are coming from now but I don't yet see how best to move forward with a naming change.

You're ok with landing this for now, though? (It just adds stringview alongside string, so it doesn't change the meaning of isData in a meaningful way.)

[tlively]

Let's land this. I'm still not happy with the state of isData, but that was true before this PR, so as long as the fuzzer is happy, there's nothing new here that I'm unhappy with.