Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Invalid WAT file format #2854

Open
Mudloop opened this issue Jun 19, 2024 · 3 comments
Open

Invalid WAT file format #2854

Mudloop opened this issue Jun 19, 2024 · 3 comments

Comments

@Mudloop
Copy link

Mudloop commented Jun 19, 2024

Bug description

I'm not sure if this is a bug with AssemblyScript, Binaryen or wat2wasm, but the generated .wat files can contain things like this :

(func $"~lib/map/Map<u32,~lib/string/String>#set:buckets" (param $this i32) (param $buckets i32)
  ..
)

When using wat2wasm, it will complain about "unexpected token ..., expected a numeric index or a name (e.g. 12 or $foo)."

This could be solved by removing the quotes, and using a minus sign instead of the commas. That should still prevent name clashes (since I don't believe a minus sign is a valid character in function or variable names), and that way wat2wam doesn't complain about it.

So in this case, that would make it :

(func $~lib/map/Map<u32-~lib/string/String>#set:buckets (param $this i32) (param $buckets i32)

It's possible that the $"" format is perfectly valid and wat2wasm just doesn't know how to deal with it, but still, probably a good idea to be compatible with commonly used tools?

Steps to reproduce

Build any project which references a generic type with more than 1 generic type parameter, ie Map<u32, string>

AssemblyScript version

0.27.27

@Mudloop Mudloop added the bug label Jun 19, 2024
@CountBleck
Copy link
Member

By the way, Binaryen might be able to convert the WAT back to Wasm, depending on whether the WAT is in the S-expression format or not.

@Mudloop
Copy link
Author

Mudloop commented Jun 19, 2024

By the way, Binaryen might be able to convert the WAT back to Wasm, depending on whether the WAT is in the S-expression format or not.

Thanks, but I just do this to deal with it (decided on a + sign, makes more sense than -) :

function replaceQuotedStrings(input: string): string {
	return input.replace(/\$"([^"]+)"/g, (_, p1) => `$${p1.replace(/,/g, '+')}`);
}

Might be breakable if there's strings that contain this pattern inside of them, but I'll deal with that if that's ever a problem. Not sure if that can even happen, but maybe if a string ends in a $ sign or something. Edit: I guess disallowing whitespace in the pattern might solve that.

@HerrCai0907
Copy link
Member

HerrCai0907 commented Nov 2, 2024

I don't think it is a bug in AS side. WAT / WASM naming custom section don't have rule to forbidden using this kinds of char.
The reason is that AS use binaryen related stuff but wat2wasm is created by wabt. There are some small difference in those two tools.

But I agree with you that it is a good idea to be compatible with commonly used tools. Maybe we can add a new option and post-process after generating WAT file.

function replaceQuotedStrings(input: string): string {
	return input.replace(/\$"([^"]+)"/g, (_, p1) => `$${p1.replace(/,/g, '+')}`);
}

Welcome to create a PR to add a new option to keep compatibility with wabt tools if you want.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants