[PATCH] D40725: Wasm entrypoint changes #3 (add --no-entry argument to LLD) APPLY AFTER D40559

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: wasm/Driver.cpp:226
+    return def;
+  // Apparently the empty string is a valid symbol name in Wasm, so we use a
+  // null string to mean "no entrypoint", *not* the empty string.
----------------
sunfish wrote:
> sbc100 wrote:
> > Empty string is not a valid symbol name.   Do you have some reason to believe it is?
> There's a test for it: https://github.com/WebAssembly/testsuite/blob/master/names.wast#L20
You guys want to ban this by the wasm spec, no? Allowing the empty string as a symbol name isn't beneficial but just confusing, I think.


================
Comment at: wasm/Driver.cpp:227
+  // Apparently the empty string is a valid symbol name in Wasm, so we use a
+  // null string to mean "no entrypoint", *not* the empty string.
+  if (Arg->getOption().getID() == OPT_no_entry)
----------------
A newly created StringRef is considered to have an empty string. We don't usually distinguish uninitialized StringRef from a StringRef that happens to have an empty string. So this distinction is too subtle. If you really need to distinguish the two, a better way of doing it is to use Optional<StringRef>.

(But I believe disallowing the empty symbol is a better approach though.)


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D40725