[PATCH] D43112: [WebAssembly] Use Symbol class heirarchy. NFC.

[Sam Clegg via Phabricator via llvm-commits]

sbc100 added inline comments.


================
Comment at: wasm/Symbols.h:88
+
+class FunctionSymbol : public Symbol {
+public:
----------------
ruiu wrote:
> This class hierarchy is interesting
> 
>   Symbol
>     FunctionSymbol
>       DefinedFunction
>       UndefinedFunction
>     GlobalSymbol
>       DefinedGlobal
>       UndefinedGlobal
>     Lazy
> 
> because in ELF and COFF, defined/undefined is a broader category than function/non-function. But I can see that your class hierarchy just represents how symbols are organized in wasm. (That's one of the reasons why I think that the current design of lld, in which all ports share only the design but not a concrete implementation. If we had to implement coff/elf/wasm/etc on all "unified" code base, that would have been extremely hard to do.)
Indeed.   It is quite major difference from ELF here.   It only occurred to be when doing this work that ELF doesn't have different undefined types.   Whereas in wasm an undefined symbol has a lot of type information (for example undefined functions carry their type signature). 

We are a little constrained by your typical OO class hierarchy here because, for example, under this scheme there is no common base class for defined or undefined symbols, not that its a huge problem.  I think this scheme is still the best options.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D43112