[llvm-commits] [llvm] r79739 - in /llvm/trunk: include/llvm/MC/MCAssembler.h lib/MC/MCAssembler.cpp lib/MC/MCMachOStreamer.cpp

[Chris Lattner]

On Aug 22, 2009, at 3:13 AM, Daniel Dunbar wrote:
> Log:
> llvm-mc/Mach-O: Sketch symbol table support.
> - The only .s syntax this honors right now is emitting labels, and  
> some parts
>   of the symbol table generation are wrong or faked.
>
> - This is enough to get nm to report such symbols... incorrectly,  
> but still.

Nice!

> +// FIXME: Same concerns as with SectionData.
> +class MCSymbolData : public ilist_node<MCSymbolData> {

Like the SectionData, I think this should be in a densemap, at least  
for now.

>   /// @}
> +  /// @name Symbol List Access
> +  /// @{
> +
> +  const SymbolDataListType &getSymbolList() const { return Symbols; }
> +  SymbolDataListType &getSymbolList() { return Symbols; }

It would be best to avoid exposing the implementation like this by  
adding lookup/insert methods to do the two operations that the symbol  
table should support.  Also why "symbol list"?  how about symbol table?

-Chris