[PATCH] D44150: [WebAssembly] Move WasmSignatureDenseMapInfo to header for reuse. NFC

Nicholas Wilson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 6 14:27:23 PST 2018


ncw added inline comments.


================
Comment at: wasm/Writer.cpp:122
   std::vector<const WasmSignature *> Types;
-  DenseMap<WasmSignature, int32_t, WasmSignatureDenseMapInfo> TypeIndices;
   std::vector<const Symbol *> ImportedSymbols;
----------------
sbc100 wrote:
> Interesting.  Is this the preferred way to do this?   Seems better I guess.
> 
> @ruiu, you know more about the llvm ADT stuff than me.  Is this how it should have been from the beginning?
In fact - why are we needing to do this in LLD at all? Surely the == operators and DenseMap traits for these fundamental types should be in the LLVM repo (at the end of `BinaryFormat/Wasm.h`) rather than here in LLD?

There's //another// mostly-but-not-quite-identical copy of this code in WasmObjectWriter (WasmFunctionTypeDenseMapInfo). So moving it into Wasm.h would make sense to me.

Are core headers like `BinaryFormat/Wasm.h` "supposed" to define container-specific headers like DenseMap traits and equality operators, for the structures they define?


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D44150





More information about the llvm-commits mailing list