Drop support for non-standard quoted function names (#6188)

We previously supported a non-standard `(func "name" ...` syntax for declaring
functions exported with the quoted name. Since that is not part of the standard
text format, drop support for it, replacing it with the standard `(func $name
(export "name") ...` syntax instead.

Also replace our other usage of the quoted form in our text output, which was
where we quoted names containing characters that are not allowed to appear in
standard names. To handle that case, adjust our output from `"$name"` to
`$"name"`, which is the standards-track way of supporting such names. Also fix
how we detect non-standard name characters to match the spec.

Update the lit test output generation script to account for these changes,
including by making the `$` prefix on names mandatory. This causes the script to
stop interpreting declarative element segments with the `(elem declare ...`
syntax as being named "declare", so prevent our generated output from regressing
by counting "declare" as a name in the script.

3 files changed, 54 insertions, 41 deletions

diff --git a/src/passes/Heap2Local.cpp b/src/passes/Heap2Local.cpp
index 0f569374b..fcf0d8886 100644
--- a/src/passes/Heap2Local.cpp
+++ b/src/passes/Heap2Local.cpp
@@ -38,7 +38,7 @@
 //
 //    (import "env" "import" (func $import (param i32) (result i32)))
 //
-//    (func "example"
+//    (func $example
 //     (local $ref (ref null $boxed-int))
 //
 //     ;; Allocate a boxed integer of 42 and save the reference to it.
diff --git a/src/passes/Print.cpp b/src/passes/Print.cpp
index 71181566d..5ea62c9f2 100644
--- a/src/passes/Print.cpp
+++ b/src/passes/Print.cpp
@@ -51,40 +51,6 @@ bool isFullForced() {
   return false;
 }
 
-std::ostream& printName(Name name, std::ostream& o) {
-  assert(name && "Cannot print an empty name");
-  // We need to quote names if they have tricky chars.
-  // TODO: This is not spec-compliant since the spec does not support quoted
-  // identifiers and has a limited set of valid idchars. We need a more robust
-  // escaping scheme here. Reusing `printEscapedString` is not sufficient,
-  // either.
-  if (name.str.find_first_of("()") == std::string_view::npos) {
-    o << '$' << name.str;
-  } else {
-    o << "\"$" << name.str << '"';
-  }
-  return o;
-}
-
-std::ostream& printMemoryName(Name name, std::ostream& o, Module* wasm) {
-  if (!wasm || wasm->memories.size() > 1) {
-    o << ' ';
-    printName(name, o);
-  }
-  return o;
-}
-
-std::ostream& printLocal(Index index, Function* func, std::ostream& o) {
-  Name name;
-  if (func) {
-    name = func->getLocalNameOrDefault(index);
-  }
-  if (!name) {
-    name = Name::fromInt(index);
-  }
-  return printName(name, o);
-}
-
 std::ostream& printEscapedString(std::ostream& os, std::string_view str) {
   os << '"';
   for (unsigned char c : str) {
@@ -119,6 +85,56 @@ std::ostream& printEscapedString(std::ostream& os, std::string_view str) {
   return os << '"';
 }
 
+// TODO: Use unicode rather than char.
+bool isIDChar(char c) {
+  if ('0' <= c && c <= '9') {
+    return true;
+  }
+  if ('A' <= c && c <= 'Z') {
+    return true;
+  }
+  if ('a' <= c && c <= 'z') {
+    return true;
+  }
+  static std::array<char, 23> otherIDChars = {
+    {'!', '#', '$', '%', '&', '\'', '*', '+', '-', '.', '/', ':',
+     '<', '=', '>', '?', '@', '\\', '^', '_', '`', '|', '~'}};
+  return std::find(otherIDChars.begin(), otherIDChars.end(), c) !=
+         otherIDChars.end();
+}
+
+std::ostream& printName(Name name, std::ostream& o) {
+  assert(name && "Cannot print an empty name");
+  // We need to quote names if they have tricky chars.
+  // TODO: This is not spec-compliant since the spec does not yet support quoted
+  // identifiers and has a limited set of valid idchars.
+  o << '$';
+  if (std::all_of(name.str.begin(), name.str.end(), isIDChar)) {
+    return o << name.str;
+  } else {
+    return printEscapedString(o, name.str);
+  }
+}
+
+std::ostream& printMemoryName(Name name, std::ostream& o, Module* wasm) {
+  if (!wasm || wasm->memories.size() > 1) {
+    o << ' ';
+    printName(name, o);
+  }
+  return o;
+}
+
+std::ostream& printLocal(Index index, Function* func, std::ostream& o) {
+  Name name;
+  if (func) {
+    name = func->getLocalNameOrDefault(index);
+  }
+  if (!name) {
+    name = Name::fromInt(index);
+  }
+  return printName(name, o);
+}
+
 // Print a name from the type section, if available. Otherwise print the type
 // normally.
 void printTypeOrName(Type type, std::ostream& o, Module* wasm) {
@@ -3266,7 +3282,8 @@ void PrintSExpression::visitModule(Module* curr) {
     printMedium(o, "(elem");
     o << " declare func";
     for (auto name : elemDeclareNames) {
-      o << " $" << name;
+      o << ' ';
+      printName(name, o);
     }
     o << ')' << maybeNewLine;
   }
diff --git a/src/wasm/wasm-s-parser.cpp b/src/wasm/wasm-s-parser.cpp
index 49cf1ccfc..42785e40e 100644
--- a/src/wasm/wasm-s-parser.cpp
+++ b/src/wasm/wasm-s-parser.cpp
@@ -1073,11 +1073,7 @@ size_t SExpressionWasmBuilder::parseFunctionNames(Element& s,
                                                   Name& exportName) {
   size_t i = 1;
   while (i < s.size() && i < 3 && s[i]->isStr()) {
-    if (s[i]->quoted()) {
-      // an export name
-      exportName = s[i]->str();
-      i++;
-    } else if (s[i]->dollared()) {
+    if (s[i]->dollared()) {
       name = s[i]->str();
       i++;
     } else {