[PATCH] D59237: [WebAssembly] Improve support for "needed" list in dylink section

Rui Ueyama via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 13 11:08:23 PDT 2019


ruiu added inline comments.


================
Comment at: lld/wasm/Writer.cpp:504
+  writeUleb128(OS, Symtab->SharedFiles.size(), "Needed");
+  for (auto *SO : Symtab->SharedFiles)
+    writeStr(OS, llvm::sys::path::filename(SO->getName()), "so name");
----------------
Maybe it is better to avoid `auto` and make it explicit as `SharedFile *`


================
Comment at: lld/wasm/Writer.cpp:505
+  for (auto *SO : Symtab->SharedFiles)
+    writeStr(OS, llvm::sys::path::filename(SO->getName()), "so name");
 }
----------------
nit: you can omit `llvm::`.


================
Comment at: llvm/lib/Object/WasmObjectFile.cpp:327
   // See https://github.com/WebAssembly/tool-conventions/blob/master/DynamicLinking.md
+  HasDylinkSection = true;
   DylinkInfo.MemorySize = readVaruint32(Ctx);
----------------
Is this dead code?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59237/new/

https://reviews.llvm.org/D59237





More information about the llvm-commits mailing list