Implement module and local names in name section (#3115)

Adds support for the module and local subsections of the name section plus the respective C and JS APIs to populate and obtain local names.

C API:
* BinaryenFunctionGetNumLocals(func)
* BinaryenFunctionHasLocalName(func, index)
* BinaryenFunctionGetLocalName(func, index)
* BinaryenFunctionSetLocalName(func, index, name)

JS API:
* Function.getNumLocals(func)
* Function.hasLocalName(func, index)
* Function.getLocalName(func, index)
* Function.setLocalName(func, index, name)

Author: dcodeIO

src/binaryen-c.cpp

@@ -3709,6 +3709,22 @@ BinaryenType BinaryenFunctionGetVar(BinaryenFunctionRef func,
   assert(index < vars.size());
   return vars[index].getID();
 }
+BinaryenIndex BinaryenFunctionGetNumLocals(BinaryenFunctionRef func) {
+  return ((Function*)func)->getNumLocals();
+}
+int BinaryenFunctionHasLocalName(BinaryenFunctionRef func,
+                                 BinaryenIndex index) {
+  return ((Function*)func)->hasLocalName(index);
+}
+const char* BinaryenFunctionGetLocalName(BinaryenFunctionRef func,
+                                         BinaryenIndex index) {
+  return ((Function*)func)->getLocalName(index).str;
+}
+void BinaryenFunctionSetLocalName(BinaryenFunctionRef func,
+                                  BinaryenIndex index,
+                                  const char* name) {
+  ((Function*)func)->setLocalName(index, name);
+}
 BinaryenExpressionRef BinaryenFunctionGetBody(BinaryenFunctionRef func) {
   return ((Function*)func)->body;
 }

src/binaryen-c.h

@@ -2236,6 +2236,19 @@ BINARYEN_API BinaryenIndex BinaryenFunctionGetNumVars(BinaryenFunctionRef func);
 // specified `Function`.
 BINARYEN_API BinaryenType BinaryenFunctionGetVar(BinaryenFunctionRef func,
                                                  BinaryenIndex index);
+// Gets the number of locals within the specified function. Includes parameters.
+BINARYEN_API BinaryenIndex
+BinaryenFunctionGetNumLocals(BinaryenFunctionRef func);
+// Tests if the local at the specified index has a name.
+BINARYEN_API int BinaryenFunctionHasLocalName(BinaryenFunctionRef func,
+                                              BinaryenIndex index);
+// Gets the name of the local at the specified index.
+BINARYEN_API const char* BinaryenFunctionGetLocalName(BinaryenFunctionRef func,
+                                                      BinaryenIndex index);
+// Sets the name of the local at the specified index.
+BINARYEN_API void BinaryenFunctionSetLocalName(BinaryenFunctionRef func,
+                                               BinaryenIndex index,
+                                               const char* name);
 // Gets the body of the specified `Function`.
 BINARYEN_API BinaryenExpressionRef
 BinaryenFunctionGetBody(BinaryenFunctionRef func);

src/js/binaryen.js-post.js

@@ -3061,18 +3061,18 @@ function makeExpressionWrapper(ownStaticMembers) {
   // inherit from Expression
   (SpecificExpression.prototype = Object.create(Expression.prototype)).constructor = SpecificExpression;
   // make own instance members
-  makeExpressionWrapperInstanceMembers(SpecificExpression.prototype, ownStaticMembers);
+  makeWrapperInstanceMembers(SpecificExpression.prototype, ownStaticMembers);
   return SpecificExpression;
 }
 
 // Makes instance members from the given static members
-function makeExpressionWrapperInstanceMembers(prototype, staticMembers) {
+function makeWrapperInstanceMembers(prototype, staticMembers, ref = 'expr') {
   Object.keys(staticMembers).forEach(memberName => {
     const member = staticMembers[memberName];
     if (typeof member === "function") {
       // Instance method calls the respective static method
       prototype[memberName] = function(...args) {
-        return this.constructor[memberName](this['expr'], ...args);
+        return this.constructor[memberName](this[ref], ...args);
       };
       // Instance accessor calls the respective static methods
       let match;
@@ -3082,10 +3082,10 @@ function makeExpressionWrapperInstanceMembers(prototype, staticMembers) {
         const setterIfAny = staticMembers["set" + memberName.substring(index)];
         Object.defineProperty(prototype, propertyName, {
           get() {
-            return member(this['expr']);
+            return member(this[ref]);
           },
           set(value) {
-            if (setterIfAny) setterIfAny(this['expr'], value);
+            if (setterIfAny) setterIfAny(this[ref], value);
             else throw Error("property '" + propertyName + "' has no setter");
           }
         });
@@ -3114,7 +3114,7 @@ Expression['finalize'] = function(expr) {
 Expression['toText'] = function(expr) {
   return Module['emitText'](expr);
 };
-makeExpressionWrapperInstanceMembers(Expression.prototype, Expression);
+makeWrapperInstanceMembers(Expression.prototype, Expression);
 Expression.prototype['valueOf'] = function() {
   return this['expr'];
 };
@@ -4281,6 +4281,42 @@ Module['TupleExtract'] = makeExpressionWrapper({
   }
 });
 
+// Function wrapper
+
+Module['Function'] = (() => {
+  function Function(func) {
+    if (!(this instanceof Function)) {
+      if (!func) return null;
+      return new Function(func);
+    }
+    if (!func) throw Error("function reference must not be null");
+    this['func'] = func;
+  }
+  Function['getName'] = function(func) {
+    return Module['_BinaryenFunctionGetName'](func);
+  };
+  Function['getNumLocals'] = function(func) {
+    return Module['_BinaryenFunctionGetNumLocals'](func);
+  };
+  Function['hasLocalName'] = function(func, index) {
+    return Boolean(Module['_BinaryenFunctionHasLocalName'](func, index));
+  };
+  Function['getLocalName'] = function(func, index) {
+    return UTF8ToString(Module['_BinaryenFunctionGetLocalName'](func, index));
+  };
+  Function['setLocalName'] = function(func, index, name) {
+    preserveStack(() => {
+      Module['_BinaryenFunctionSetLocalName'](func, index, strToStack(name));
+    });
+  };
+  // TODO: add more methods
+  makeWrapperInstanceMembers(Function.prototype, Function, 'func');
+  Function.prototype['valueOf'] = function() {
+    return this['func'];
+  };
+  return Function;
+})();
+
 // Additional customizations
 
 Module['exit'] = function(status) {

src/passes/Print.cpp

@@ -2398,6 +2398,10 @@ struct PrintSExpression : public OverriddenVisitor<PrintSExpression> {
     currModule = curr;
     o << '(';
     printMajor(o, "module");
+    if (curr->name.is()) {
+      o << ' ';
+      printName(curr->name, o);
+    }
     incIndent();
     std::vector<Signature> signatures;
     std::unordered_map<Signature, Index> indices;

src/tools/wasm-shell.cpp

@@ -304,9 +304,9 @@ int main(int argc, const char* argv[]) {
         std::cerr << "BUILDING MODULE [line: " << curr.line << "]\n";
         Colors::normal(std::cerr);
         auto module = wasm::make_unique<Module>();
-        Name moduleName;
         auto builder = wasm::make_unique<SExpressionWasmBuilder>(
-          *module, *root[i], IRProfile::Normal, &moduleName);
+          *module, *root[i], IRProfile::Normal);
+        auto moduleName = module->name;
         builders[moduleName].swap(builder);
         modules[moduleName].swap(module);
         i++;

src/wasm-binary.h

@@ -386,6 +386,7 @@ extern const char* MultivalueFeature;
 extern const char* AnyrefFeature;
 
 enum Subsection {
+  NameModule = 0,
   NameFunction = 1,
   NameLocal = 2,
 };

src/wasm-s-parser.h

@@ -126,10 +126,7 @@ class SExpressionWasmBuilder {
 
 public:
   // Assumes control of and modifies the input.
-  SExpressionWasmBuilder(Module& wasm,
-                         Element& module,
-                         IRProfile profile,
-                         Name* moduleName = nullptr);
+  SExpressionWasmBuilder(Module& wasm, Element& module, IRProfile profile);
 
 private:
   // pre-parse types and function definitions, so we know function return types

src/wasm.h

@@ -1263,7 +1263,7 @@ using StackIR = std::vector<StackInst*>;
 class Function : public Importable {
 public:
   Name name;
-  Signature sig;          // parameters and return value
+  Signature sig; // parameters and return value
   IRProfile profile = IRProfile::Normal;
   std::vector<Type> vars; // non-param locals
 
@@ -1328,6 +1328,7 @@ class Function : public Importable {
   Name getLocalNameOrGeneric(Index index);
 
   bool hasLocalName(Index index) const;
+  void setLocalName(Index index, Name name);
 
   void clearNames();
   void clearDebugInfo();
@@ -1502,6 +1503,9 @@ class Module {
   FeatureSet features = FeatureSet::MVP;
   bool hasFeaturesSection = false;
 
+  // Module name, if specified. Serves a documentary role only.
+  Name name;
+
   MixedArena allocator;
 
 private:

src/wasm/wasm-binary.cpp

@@ -555,20 +555,79 @@ void WasmBinaryWriter::writeNames() {
   BYN_TRACE("== writeNames\n");
   auto start = startSection(BinaryConsts::Section::User);
   writeInlineString(BinaryConsts::UserSections::Name);
-  auto substart =
-    startSubsection(BinaryConsts::UserSections::Subsection::NameFunction);
-  o << U32LEB(indexes.functionIndexes.size());
-  Index emitted = 0;
-  auto add = [&](Function* curr) {
-    o << U32LEB(emitted);
-    writeEscapedName(curr->name.str);
-    emitted++;
-  };
-  ModuleUtils::iterImportedFunctions(*wasm, add);
-  ModuleUtils::iterDefinedFunctions(*wasm, add);
-  assert(emitted == indexes.functionIndexes.size());
-  finishSubsection(substart);
-  /* TODO: locals */
+
+  // module name
+  if (wasm->name.is()) {
+    auto substart =
+      startSubsection(BinaryConsts::UserSections::Subsection::NameModule);
+    writeEscapedName(wasm->name.str);
+    finishSubsection(substart);
+  }
+
+  // function names
+  {
+    auto substart =
+      startSubsection(BinaryConsts::UserSections::Subsection::NameFunction);
+    o << U32LEB(indexes.functionIndexes.size());
+    Index emitted = 0;
+    auto add = [&](Function* curr) {
+      o << U32LEB(emitted);
+      writeEscapedName(curr->name.str);
+      emitted++;
+    };
+    ModuleUtils::iterImportedFunctions(*wasm, add);
+    ModuleUtils::iterDefinedFunctions(*wasm, add);
+    assert(emitted == indexes.functionIndexes.size());
+    finishSubsection(substart);
+  }
+
+  // local names
+  {
+    // Find all functions with at least one local name and only emit the
+    // subsection if there is at least one.
+    std::vector<std::pair<Index, Function*>> functionsWithLocalNames;
+    Index checked = 0;
+    auto check = [&](Function* curr) {
+      auto numLocals = curr->getNumLocals();
+      for (Index i = 0; i < numLocals; ++i) {
+        if (curr->hasLocalName(i)) {
+          functionsWithLocalNames.push_back({checked, curr});
+          break;
+        }
+      }
+      checked++;
+    };
+    ModuleUtils::iterImportedFunctions(*wasm, check);
+    ModuleUtils::iterDefinedFunctions(*wasm, check);
+    assert(checked == indexes.functionIndexes.size());
+    if (functionsWithLocalNames.size() > 0) {
+      // Otherwise emit those functions but only include locals with a name.
+      auto substart =
+        startSubsection(BinaryConsts::UserSections::Subsection::NameLocal);
+      o << U32LEB(functionsWithLocalNames.size());
+      Index emitted = 0;
+      for (auto& indexedFunc : functionsWithLocalNames) {
+        std::vector<std::pair<Index, Name>> localsWithNames;
+        auto numLocals = indexedFunc.second->getNumLocals();
+        for (Index i = 0; i < numLocals; ++i) {
+          if (indexedFunc.second->hasLocalName(i)) {
+            localsWithNames.push_back({i, indexedFunc.second->getLocalName(i)});
+          }
+        }
+        assert(localsWithNames.size());
+        o << U32LEB(indexedFunc.first);
+        o << U32LEB(localsWithNames.size());
+        for (auto& indexedLocal : localsWithNames) {
+          o << U32LEB(indexedLocal.first);
+          writeEscapedName(indexedLocal.second.str);
+        }
+        emitted++;
+      }
+      assert(emitted == functionsWithLocalNames.size());
+      finishSubsection(substart);
+    }
+  }
+
   finishSection(start);
 }
 
@@ -2126,33 +2185,72 @@ void WasmBinaryBuilder::readNames(size_t payloadLen) {
     auto nameType = getU32LEB();
     auto subsectionSize = getU32LEB();
     auto subsectionPos = pos;
-    if (nameType != BinaryConsts::UserSections::Subsection::NameFunction) {
-      // TODO: locals
-      std::cerr << "warning: unknown name subsection at " << pos << std::endl;
-      pos = subsectionPos + subsectionSize;
-      continue;
-    }
-    auto num = getU32LEB();
-    std::set<Name> usedNames;
-    for (size_t i = 0; i < num; i++) {
-      auto index = getU32LEB();
-      auto rawName = getInlineString();
-      rawName = escape(rawName);
-      auto name = rawName;
-      // De-duplicate names by appending .1, .2, etc.
-      for (int i = 1; !usedNames.insert(name).second; ++i) {
-        name = rawName.str + std::string(".") + std::to_string(i);
+    if (nameType == BinaryConsts::UserSections::Subsection::NameModule) {
+      wasm.name = getInlineString();
+    } else if (nameType ==
+               BinaryConsts::UserSections::Subsection::NameFunction) {
+      auto num = getU32LEB();
+      std::set<Name> usedNames;
+      for (size_t i = 0; i < num; i++) {
+        auto index = getU32LEB();
+        auto rawName = getInlineString();
+        rawName = escape(rawName);
+        auto name = rawName;
+        // De-duplicate names by appending .1, .2, etc.
+        for (int i = 1; !usedNames.insert(name).second; ++i) {
+          name = rawName.str + std::string(".") + std::to_string(i);
+        }
+        // note: we silently ignore errors here, as name section errors
+        //       are not fatal. should we warn?
+        auto numFunctionImports = functionImports.size();
+        if (index < numFunctionImports) {
+          functionImports[index]->name = name;
+        } else if (index - numFunctionImports < functions.size()) {
+          functions[index - numFunctionImports]->name = name;
+        } else {
+          std::cerr << "warning: function index out of bounds in name section, "
+                       "function subsection: "
+                    << std::string(name.str) << " at index "
+                    << std::to_string(index) << std::endl;
+        }
       }
-      // note: we silently ignore errors here, as name section errors
-      //       are not fatal. should we warn?
+    } else if (nameType == BinaryConsts::UserSections::Subsection::NameLocal) {
+      auto numFuncs = getU32LEB();
       auto numFunctionImports = functionImports.size();
-      if (index < numFunctionImports) {
-        functionImports[index]->name = name;
-      } else if (index - numFunctionImports < functions.size()) {
-        functions[index - numFunctionImports]->name = name;
-      } else {
-        throwError("index out of bounds: " + std::string(name.str));
+      for (size_t i = 0; i < numFuncs; i++) {
+        auto funcIndex = getU32LEB();
+        Function* func = nullptr;
+        if (funcIndex < numFunctionImports) {
+          func = functionImports[funcIndex];
+        } else if (funcIndex - numFunctionImports < functions.size()) {
+          func = functions[funcIndex - numFunctionImports];
+        } else {
+          std::cerr
+            << "warning: function index out of bounds in name section, local "
+               "subsection: "
+            << std::to_string(funcIndex) << std::endl;
+        }
+        auto numLocals = getU32LEB();
+        for (size_t j = 0; j < numLocals; j++) {
+          auto localIndex = getU32LEB();
+          auto localName = getInlineString();
+          if (!func) {
+            continue; // read and discard in case of prior error
+          }
+          if (localIndex < func->getNumLocals()) {
+            func->localNames[localIndex] = localName;
+          } else {
+            std::cerr << "warning: local index out of bounds in name "
+                         "section, local subsection: "
+                      << std::string(localName.str) << " at index "
+                      << std::to_string(localIndex) << " in function "
+                      << std::string(func->name.str) << std::endl;
+          }
+        }
       }
+    } else {
+      std::cerr << "warning: unknown name subsection at " << pos << std::endl;
+      pos = subsectionPos + subsectionSize;
     }
     if (pos != subsectionPos + subsectionSize) {
       throwError("bad names subsection position change");