[PATCH] D42095: [WebAssembly] Output provisional table to match LLD relocatable output

Nicholas Wilson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 31 11:25:56 PST 2018


ncw added inline comments.


================
Comment at: lib/MC/WasmObjectWriter.cpp:502
-    if (!IndirectSymbolIndices.count(RelEntry.Symbol))
-      report_fatal_error("symbol not found in table index space: " +
-                         RelEntry.Symbol->getName());
----------------
sbc100 wrote:
> ncw wrote:
> > sbc100 wrote:
> > > I guess this error was removed because it should never happen right?
> > > 
> > > And converting it to an assert wouldn't be very useful?
> > > 
> > Hmm, maybe it would be better to keep the check. In fact, there are several places where we do a check using "!thing.count()", when we could instead be using "find" to do the lookup only once. That would be ideal I guess, maybe with "findOrAbort" static helper function.
> Doesn't std::vector::operator[] effectively give us the "findOrAbort" semantics?
> 
> I think I originally added this nice errors to aid debugging, but if they shouldn't ever  occur in practice then we should probably at most use a single assert (and perhaps just really on std::vector to blow up for us?)
I think it's actually a map, so it currently has "insert null if not present" semantics. So there will be a null-dereference crash which effectively an assert.

Feel free to add an assert when committing - or I can update the diff if you'd prefer?

What at lot of trouble over a small change, I'm sorry about this.


Repository:
  rL LLVM

https://reviews.llvm.org/D42095





More information about the llvm-commits mailing list