[PATCH] D97600: [lld-macho] Implement options -rename_section -rename_segment

[Jez Ng via Phabricator via llvm-commits]

int3 added inline comments.


================
Comment at: lld/MachO/Config.h:29-30
+using NamePair = std::pair<llvm::StringRef, llvm::StringRef>;
+using SectionRenameMap = std::map<NamePair, NamePair>;
+using SegmentRenameMap = std::map<llvm::StringRef, llvm::StringRef>;
+
----------------
gkm wrote:
> int3 wrote:
> > try to avoid `std::map` in general (as per the [[ https://llvm.org/docs/ProgrammersManual.html#map-like-containers-std-map-densemap-etc | LLVM programmer's manual ]]). In this case we could use a DenseMap of NamePair and a DenseMap of CachedHashStringRef respectively
> Why `CachedHashStringRef` ? Is that supposed to give us some code sharing, at the expense of clarity?
It saves on recomputation of the hash. Not that it matters here since this map shouldn't part of a hot loop, but I just use it everywhere we need a map of string keys, so I don't have to think about whether performance is important in a particular case.

There's also StringMap, but that allocates and owns its own strings, which for LLD isn't really optimal since our strings are mostly interned / not deallocated till the end of the process.


================
Comment at: lld/test/MachO/rename.s:33
+# CHECK-NEXT: sectname __from_seg
+# CHECK-NOT: segname __FROM_SEG
+# CHECK-NEXT: segname __TO_SEG
----------------



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97600