[PATCH] D131290: [llvm-objdump] Create name for fake sections

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/include/llvm/Object/ELF.h:185
   std::vector<Elf_Shdr> FakeSections;
+  std::vector<char> FakeSectionStrings;
 
----------------
MaskRay wrote:
> namhyung wrote:
> > MaskRay wrote:
> > > Use `SmallString<0> FakeSectionStrings`.
> > > 
> > > Prefer `SmallString` because `std::string` has a small string optimization which isn't useful for this case.
> > But I think it'd be better to use StringTableBuilder as @jhenderson said.
> I think `StringTableBuilder` just adds complexity/overhead in this case. `StringTableBuilder` cannot deduplicate any string here (and the code does not need to optimize  for size).
I disagree that it adds complexity. It seems more intuitive to do `StrTab.add(someString)` than `std::copy(someString.begin(), someString.end(), std::back_inserter(StrTab));` followed by a null terminator addition. I do agree that the optimizations are unnecessary. Maybe there's room for a "simple" string builder class that does none of the deduplication/optimization?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131290