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

[Heejin Ahn via Phabricator via llvm-commits]

aheejin 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; }
----------------
dschuff wrote:
> 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.
I wanted to use the same terms with at least with MC, because they are the reader and writer for the same object file format. But anyway, if this direction does not look better, I can change the MC part to match this part to use 'type's.


Repository:
  rL LLVM

https://reviews.llvm.org/D54689