[PATCH] D135898: [wasm-ld] Allow importing/exporting the output module's memory with arbitrary names

[Sam Clegg via Phabricator via llvm-commits]

sbc100 added a comment.

Seems reasonable.  Does the same thing need to be don't for the indirect function table?



================
Comment at: lld/wasm/Driver.cpp:538
+      config->memoryImport =
+          std::pair<llvm::StringRef, llvm::StringRef>("env", "memory");
+    }
----------------
why not use `defaultModule` and `memoryName` here?


================
Comment at: lld/wasm/Driver.cpp:544
+  // If neither export-memory nor import-memory is specified, default to
+  // exporting memory as '"memory"'.
+  if (!config->memoryExport.hasValue() && !config->memoryImport.hasValue()) {
----------------
If you use `memoryName` below maybe change this to ` default to exporting memory under its default name.`?


================
Comment at: lld/wasm/Driver.cpp:546
+  if (!config->memoryExport.hasValue() && !config->memoryImport.hasValue()) {
+    config->memoryExport = "memory";
+  }
----------------
Why not use `memoryName` here?


================
Comment at: lld/wasm/SyntheticSections.cpp:237
 
-  if (config->importMemory) {
+  if (config->memoryImport.hasValue()) {
     WasmImport import;
----------------
Does llvm::Optional no coerce to boolean automatically here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135898