[PATCH] D54689: [WebAssembly] Rename function types to signatures

[Derek Schuff via Phabricator via llvm-commits]

dschuff added inline comments.


================
Comment at: include/llvm/Object/Wasm.h:134
+  ArrayRef<wasm::WasmSignature> signatures() const { return Signatures; }
+  ArrayRef<uint32_t> functionSigIndices() const { return FunctionSigIndices; }
   ArrayRef<wasm::WasmImport> imports() const { return Imports; }
----------------
aheejin wrote:
> sbc100 wrote:
> > Maybe just functionSignatures?
> Then we end up calling both `WasmSignature` objects and indices (`uint32_t`) as 'Signatures'. One of the main purpose of this patch is to distinguish them. If you think that makes things more confusing, I think I can drop this patch.
In most parts of the compiler I think we want to be quite explicit about what kind of kind of type we are talking about because there are so many (e.g. `MVT`, `wasm::ValueType`, `WasmSignature` etc). Keeping those straight globally might be worth more than lining up exactly with the spec in the object file format. OTOH if this code is pretty well separated from e.g. MI (yes), and MC (maybe?) and every construct in here unambiguously lines up with something from the wasm object file format and not one of those other sets of abstractions, then we don't have to worry as much about that.


Repository:
  rL LLVM

https://reviews.llvm.org/D54689