[PATCH] D43147: [WebAssembly] Add first class symbol table to wasm objects

[Sam Clegg via Phabricator via llvm-commits]

sbc100 added inline comments.


================
Comment at: include/llvm/BinaryFormat/Wasm.h:259
 
+const unsigned WASM_SYMBOL_UNDEFINED          = 0x100;
+
----------------
ncw wrote:
> Should this be renumbered down to 0x8 or 0x10, to use the next available bit, now that it's written out?
I thought maybe it would be good idea to leave gap in case we want another bit for WASM_SYMBOL_VISIBILITY_MASK... but maybe in that case we should have make the existing masks bigger to start with. 

Should I add an extra bit to BINDING_MASK and VISIBILITY_MASK for future use?  Maybe no need?


================
Comment at: include/llvm/MC/MCSymbolWasm.h:23
+    GLOBAL
+  };
+
----------------
ncw wrote:
> I thought we weren't duplicating this enum, can't it be a wasm::WasmSymbolType? Would also save the function in WasmObjectWriter to translate between the two enums.
Hmm.. I guess I didn't get the lastest version of your change :(


Repository:
  rL LLVM

https://reviews.llvm.org/D43147