[PATCH] D40724: Wasm entrypoint changes #1 (add --undefined argument to LLD) APPLY AFTER D40690

[Sam Clegg via Phabricator via llvm-commits]

sbc100 added inline comments.


================
Comment at: test/wasm/load-undefined.ll:10
+; RUN: obj2yaml %t.wasm | FileCheck %s
+
+
----------------
Could you add test for when the undefined symbol is not found?   i.e. -u foo should cause a certain linker error?  (unless we decide it doesn't, see below.)

(You can just add that case to within this file I think).


================
Comment at: wasm/Driver.cpp:292
+    for (StringRef S : Undefined)
+      addSyntheticUndefinedFunction(S, nullptr);
+
----------------
Hmm.. if we allow function symbols to have a null type I'm afraid we won't be able to represent them in the output object (since we can have an import or and export without a function type.  Perhaps this is OK if we know these symbols will never appear in the output, but I'm worried that --allow-undefined will allow them to appear.

Also, I was experimenting with --undefined with GNU ld.  The behaviour seems to be that if the symbol can't be found it simply results in an undefined symbol in the final output, not a link error.  Which I find strange, but maybe we can to mimik that?


================
Comment at: wasm/Symbols.h:72
 
+  bool hasFunctionType() const { return FunctionType; }
   const WasmSignature &getFunctionType() const;
----------------
I think its more clear if you explicitly compare with nullptr here.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D40724