[PATCH] D63309: [llvm-objcopy][MachO] Rebuild the symbol/string table in the writer

[Alexander Shaposhnikov via Phabricator via llvm-commits]

alexshap added inline comments.


================
Comment at: llvm/tools/llvm-objcopy/MachO/MachOReader.cpp:173
+  SymbolEntry SE;
+  SE.Name = StringRef(&StrTable.data()[nlist.n_strx]).str();
+  SE.n_type = nlist.n_type;
----------------

   StringRef(StrTable.data() + nlist.n_strx).str()


================
Comment at: llvm/tools/llvm-objcopy/MachO/MachOReader.cpp:186
+        (MachOObj.is64Bit()
+             ? constructSymbolEntry<MachO::nlist_64>(
+                   StrTable,
----------------
i think you don't need the explicit specification of the template parameter here, it'll be deduced from the type of the second argument


================
Comment at: llvm/tools/llvm-objcopy/MachO/Object.cpp:10
+  assert(Index < Symbols.size() && "invalid symbol index");
+  return const_cast<SymbolEntry *>(Symbols[Index].get());
+}
----------------
khm, why is this const_cast needed here ?


================
Comment at: llvm/tools/llvm-objcopy/MachO/Object.h:100
+
+  SymbolEntry *getSymbolByIndex(uint32_t Index);
 };
----------------

  SymbolEntry *getSymbolByIndex(uint32_t Index) const;


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

https://reviews.llvm.org/D63309