[Nishuuzz]

wasm2wat --generate-names can write a module that wat2wasm then refuses:

$ wasm2wat --generate-names t.wasm -o t.wat
$ wat2wasm t.wat
t.wat:4:4: error: redefinition of function "$a_b"
  (func $a_b (type $t0))
   ^^^^

The module exports three functions named "a b", "a,b" and "a(b", and all
three come out as $a_b.

Why

MaybeUseAndBindName() derives a name from an import or export name and loops
on a disambiguating suffix until the result is not already bound:

    unsigned disambiguator = 0;
    while (true) {
      GenerateName(name, kInvalidIndex, disambiguator, str);
      if (!bindings->contains(*str)) {
        bindings->emplace(*str, Binding(index));
        break;
      }
      disambiguator++;
    }

WatWriter::WriteName() then replaces every character a name cannot contain
with _. So the loop compares $a b, $a,b and $a(b, finds no clash and
never applies a suffix, and the writer turns all three into $a_b. The
disambiguator cannot disambiguate, because the string it tests is not the
string that gets written.

names.wast in the spec testsuite runs into this: 512 of its functions are
written as $___, 242 as $____ and 86 as $__, and the output does not
assemble.

The change

Move the replacement out of WriteName() into MakeValidWatName(), declared
next to WriteWat() since it is part of what the writer guarantees about its
output, and call it in the loop before the comparison. The suffix is then
applied to exactly the names that would otherwise be written the same.

The three exports above now come out as $a_b, $a_b_1 and $a_b_2, and the
output assembles.

What this does not cover

A name that came from the binary's name section is used as-is rather than
generated here, so two of those differing only in these characters still
collide, and this affects plain wasm2wat as well as --generate-names:

$ cat t2.txt                       # gen-wasm, 57 byte module
section("name") {
  section(NAME_FUNCTION) {
    func_count[3]
    index[0] str("a b")
    index[1] str("a,b")
    index[2] str("a(b")
  }
}
$ wasm2wat t2.wasm
(module
  (type (;0;) (func))
  (func $a_b (type 0))
  (func $a_b (type 0))
  (func $a_b (type 0))
  ...
$ wat2wasm t2.wat
error: redefinition of function "$a_b"

A reference is ambiguous too: with a fourth function calling the second, the
output contains call $a_b with two definitions of that name.

I left that alone deliberately. Fixing it means renaming entities the module
itself named, and doing it before ApplyNames() runs so the vars pick the new
names up, which is a different and more visible change than correcting a
disambiguation loop that cannot currently work. Happy to follow up with it, or
open it as an issue instead, whichever you prefer.

Verification

test/binary/generate-names-invalid-chars.txt covers the generated-name case.
On current main it fails with:

   (func $a_b (type $t0))
-  (func $a_b_1 (type $t0))
-  (func $a_b_2 (type $t0))
+  (func $a_b (type $t0))
+  (func $a_b (type $t0))
   (export "a b" (func $a_b))
-  (export "a,b" (func $a_b_1))
-  (export "a(b" (func $a_b_2)))
+  (export "a,b" (func $a_b))
+  (export "a(b" (func $a_b)))

and passes with the change. I also checked that the expected output assembles,
which is the property that was actually broken.

For the suite I compared against a baseline rather than reading the output,
since wat2wasm_stdout.txt fails on unmodified main on this machine for its
own reasons. Unmodified main fails that one test; with the change it is still
the only failure, so nothing moved. clang-format --dry-run --Werror is clean
on the three changed files.

How I ran into it: I was sweeping wat-desugar over everything wast2json
extracts from third_party/testsuite, checking that desugaring preserves what
a module assembles to and is idempotent, across its output options. All 1731
modules pass both properties for no options, --fold-exprs, --inline-imports
and the two combined. --generate-names was the one option that produced text
which no longer assembles, on one module, which turned out to be this.