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

[Nicholas Wilson via Phabricator via llvm-commits]

ncw added inline comments.


================
Comment at: wasm/Writer.cpp:696
         continue;
-      if (!Sym->isLive())
-        return;
+      assert(Sym->isLive());
       Sym->setOutputSymbolIndex(SymbolIndex++);
----------------
sbc100 wrote:
> 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.
I'll add a comment then, so we don't have a dead branch. As it is (with "return" rather than "continue") it needs to be changed //somehow//. Thanks!


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D44146