[PATCH] D41954: [WebAssembly] Add symbol table to LLVM, 1/2

[Sam Clegg via Phabricator via llvm-commits]

sbc100 added inline comments.


================
Comment at: lib/MC/WasmObjectWriter.cpp:1049
+      SymbolIndices[&WS] = NumSymbols++;
+      AddSymbol(WS);
     }
----------------
Can  `SymbolIndices[&WS] = NumSymbols++;` be moved in the AddSymbol?  That would seem logical to me.


================
Comment at: lib/MC/WasmObjectWriter.cpp:1103
 
-    unsigned Index;
+    unsigned WasmIndex;
 
----------------
I don't like the term WasmIndex here.  Can we just revert this or come up with a better name?   Looks like its actially used as the Export.Index below, so maybe ExportIndex?   Just reverting to Index seems reasonable though.


================
Comment at: lib/MC/WasmObjectWriter.cpp:1138
 
-      if (!WS.isDefined())
-        continue;
+      if (WS.isDefined()) {
+        if (!WS.getSize())
----------------
Revert this indentation change?  Seems like `continue;` is still what we want.


Repository:
  rL LLVM

https://reviews.llvm.org/D41954