[PATCH] D76742: [lld] Add basic symbol table output
Kellie Medlin via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Apr 2 09:44:29 PDT 2020
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
More information about the llvm-commits
mailing list