[PATCH] D76742: [lld] Add basic symbol table output

[Kellie Medlin via Phabricator via llvm-commits]

Ktwu marked an inline comment as done.
Ktwu added a comment.

> My understanding of this patch is that this creates section contents early. But can you avoid doing that?

Of course!



================
Comment at: lld/MachO/Writer.cpp:396
+  for (Symbol *sym : symtab->getSymbols()) {
+    uint8_t n_type = N_UNDF;
+    uint8_t n_sect = NO_SECT;
----------------
int3 wrote:
> smeenai wrote:
> > Nit: variable naming
> I wonder if we should just drop the `n_` prefix for internal variable names, i.e. just `strx` instead of `nStrx`. Not really sure why `n_` was added in the first place, but it seems like some kind of ad-hoc namespace convention? (Not that it really makes sense for struct members...) In any case it doesn't seem to add much to code readability
I think it helps map the value to the intended struct name; if I see a variable with this nPattern naming, it's easier to understand that the value is ultimately intended for use within the symbol table given the documented struct variable names.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76742/new/

https://reviews.llvm.org/D76742