[PATCH] D43264: [WebAssembly] Add explicit symbol table

[Nicholas Wilson via Phabricator via llvm-commits]

ncw added inline comments.


================
Comment at: wasm/Symbols.h:72-73
 
+  // Indicates that this symbol will be included in the final image. Only valid
+  // after calling markLive.
+  bool isLive() const;
----------------
OK, I'm happy with doing GC for globals in a separate place, if that's what you prefer... the isLive method is good. The comment here isn't accurate though (pedantic complaint this!) - "For function/data symbols, only valid after calling markLive; for global symbols only valid after global GC has been done".


================
Comment at: wasm/Writer.cpp:816
+  uint32_t GlobalIndex = NumImportedGlobals + InputGlobals.size();
+  auto AddDefinedGlobal = [&](InputGlobal *Global) {
+    if (Global->Live) {
----------------
(I guess this should be AddInputGlobal now)


================
Comment at: wasm/Writer.cpp:898
       CtorFunctionBody.size());
-  SyntheticFunction *F = make<SyntheticFunction>(Signature, BodyArray,
+  SyntheticFunction *F = make<SyntheticFunction>(*Signature, BodyArray,
                                                  WasmSym::CallCtors->getName());
----------------
It would be neater to give SyntheticFunction a std::string member, and call its constructor with `std::move(CtorFunctionBody)` so that the SyntheticFunction owns the synthetic body. Can do post-merge.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D43264