Implement StringConcat in the interpreter.

[rluble]

No description provided.

[kripken]

Thanks!

[gkdn]

Don't we need to pay special care around concat of two invalid UTF-16 strings? (e.g. with unmatched surrogate pairs)
Maybe we could add some tests around it.

[kripken]

Don't we need to pay special care around concat of two invalid UTF-16 strings?

I am actually not sure what the spec says about that. It's worth looking into, though it could be left as a TODO from my perspective.

[rluble]

Don't we need to pay special care around concat of two invalid UTF-16 strings?

I am actually not sure what the spec says about that. It's worth looking into, though it could be left as a TODO from my perspective.

So I couldn't find detailed specification for the behavior of concat other than the documentations for String.concat which basically says that the concatted string corresponds to the concatenation of the char representation; seems quite vague. Tested the behavior and in fact it is just the concatenation of the WTF-16 chars, so you can get a valid UTF-16 by concatenating invalid UTF-16 parts. This is both in JS and Java.

Do the Literals in the binaryen representation correspond to WTF-16 chars or to UTF-8 bytes?

[kripken]

Do the Literals in the binaryen representation correspond to WTF-16 chars or to UTF-8 bytes?

I believe the intention was that they correspond to bytes, though that was the plan when we intended to support more than JS strings (as the stringref proposal did). If all we need to support is WTF-16 for JS then perhaps we can simplify that.

Tested the behavior and in fact it is just the concatenation of the WTF-16 chars, so you can get a valid UTF-16 by concatenating invalid UTF-16 parts. This is both in JS and Java.

Thanks, that makes sense. That makes me think that maybe a good way to get coverage here is to find the JS tests for String.concat and port them to wasm. Hopefully they are in a convenient form in test262. In any case, let's leave that for later; for now all the recent PRs at least handle the simple ascii cases properly.

[kripken]

(CI / lint seems to find some errors here which need to be fixed before landing - whitespace?)

[rluble]

Still trying to get used to the workflow. Do you use the git clang-format to format the sources? no parameters? I don't think my last push fixed the formatting.

[kripken]

Doing git clang-format origin/main should work. You can also apply the diff from CI though, or fix it up manually.

[kripken]

To do it manually, copy the diff from CI which is

diff --git a/src/wasm-interpreter.h b/src/wasm-interpreter.h
index 99733bc64..13ac19d92 [10](https://github.com/WebAssembly/binaryen/actions/runs/8290790765/job/22689511066?pr=6403#step:6:11)0644
--- a/src/wasm-interpreter.h
+++ b/src/wasm-interpreter.h
@@ -1919,7 +1919,7 @@ public:
     }
     return Literal(int32_t(data->values.size()));
   }
-  Flow visitStringConcat(StringConcat* curr) { 
+  Flow visitStringConcat(StringConcat* curr) {
     NOTE_ENTER("StringConcat");
     Flow flow = visit(curr->left);
     if (flow.breaking()) {
@@ -[19](https://github.com/WebAssembly/binaryen/actions/runs/8290790765/job/22689511066?pr=6403#step:6:20)86,7 +1986,7 @@ public:
 
     return Literal(int32_t(refData->values.size()));
   }
- 
+
   Flow visitStringEq(StringEq* curr) {
     NOTE_ENTER("StringEq");
     Flow flow = visit(curr->left);

into a local file, say a.diff, and then do git apply a.diff (from the root dir of the repo).