[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