[PATCH] D44146: [WebAssembly] Remove duplicated line of code and unreachable check. NFC

[Sam Clegg via Phabricator via llvm-commits]

sbc100 accepted this revision.
sbc100 added a comment.
This revision is now accepted and ready to land.

lgtm, with commets.



================
Comment at: wasm/Writer.cpp:696
         continue;
-      if (!Sym->isLive())
-        return;
+      assert(Sym->isLive());
       Sym->setOutputSymbolIndex(SymbolIndex++);
----------------
ncw wrote:
> sbc100 wrote:
> > But this isn't iterating the symbol table, its iterating all symbols in all the objects. Surely some of them can be non-live.  I don't quite understand why we not hitting this case in some unit test.
> The entire method `Writer::assignSymtab` has an early-exit for non-relocatable output. So all symbols are always Live when generating a symtab, since that's the relocatable case (and GC and relocatable are exclusive).
I see.  That was non-obvious to me, so perhaps at least comment?

This makes two assumptions: 
1. liveness is only ever set by GC
2. GC never applies in relocatable mode.

I guess as long as the assert is here we will know if we ever violate those assumption, but I'm on the fence as to weather to just leave the check in.  I'd be happy with a comment here explaining the assumptions I think.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D44146