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

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added a comment.

As to the use of the placement new, it is an intentional design choice that symbols are trivially destructible. We shouldn't add anything that needs a desturctor to Symbol for performance reasons. We sometimes allocate literally millions of symbols, and we don't want to call destructors million times. Also, even if we had a destructor for Symbol, they wouldn't be called because we usually use _exit to terminate the process as quickly as possible, without calling the destructor of objects that are used throughout the linking process. lld may not look like very "object-oriented", and that's intentional.

As to the union type, we don't really use it as a union. That is a convenient way to allocate a memory that is large enough to hold any Symbol-derived type, but we don't access any member of it. As you wrote, maybe we should make it explicit by changing `(Symbol *)make<SymbolUnion>()` to `reinterpret_cast<Symbol *>(make<SymbolUnion>())` (I actually wrote that cast with reinterpret_cast in mind, without noticing that there's other way of interpreting it), or if the existence of the union causes confusion like that, we should remove SymbolUnion and define `char[maximum symbol size]` for a symbol instead. The point is that the union isn't important. We just want to allocate blank memory for symbols. We identify object type using LLVM's classof mechanism, and I think that works just fine.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D43112