[PATCH] D57909: [WebAssembly] Don't generate invalid modules when function signatures mismatch

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: lld/wasm/SymbolTable.cpp:130
+                       const WasmSignature *NewSig) {
   auto ExistingFunction = dyn_cast<FunctionSymbol>(Existing);
   if (!ExistingFunction) {
----------------
Could you add a short comment here saying that if one symbol is a function and the other is not a function, it is always a function type error?


================
Comment at: lld/wasm/SymbolTable.cpp:130-134
   auto ExistingFunction = dyn_cast<FunctionSymbol>(Existing);
   if (!ExistingFunction) {
     reportTypeError(Existing, File, WASM_SYMBOL_TYPE_FUNCTION);
-    return;
+    return true;
   }
----------------
ruiu wrote:
> Could you add a short comment here saying that if one symbol is a function and the other is not a function, it is always a function type error?
It looks like this function does two different things:

  1. verifying that both symbols are function symbols
  2. verifying that the two functions have the same type signature

a boolean return value doesn't make much sense for (1).

I'd remove this part of code from this function and move that error check to the caller.


================
Comment at: lld/wasm/Symbols.h:76
 
+  void setName(StringRef S) { Name = S; }
+
----------------
At this point we probably should simply make `Name` a public member, as it has both set and get accessors.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57909