[PATCH] D81717: [llvm/Object] Reimplment basic_symbol_iterator in TapiFile

[Cyndy Ishida via Phabricator via llvm-commits]

cishida accepted this revision.
cishida added a comment.
This revision is now accepted and ready to land.

Thanks for fixing this! one nit but LGTM



================
Comment at: llvm/lib/Object/TapiFile.cpp:81
 Error TapiFile::printSymbolName(raw_ostream &OS, DataRefImpl DRI) const {
-  const auto *Sym = reinterpret_cast<const Symbol *>(DRI.p);
-  OS << Sym->Prefix << Sym->Name;
+  assert(DRI.d.a < Symbols.size());
+  const Symbol &Sym = Symbols[DRI.d.a];
----------------
nit: maybe add a message to notify an out of bounds access? 


================
Comment at: llvm/lib/Object/TapiFile.cpp:88
 Expected<uint32_t> TapiFile::getSymbolFlags(DataRefImpl DRI) const {
-  const auto *Sym = reinterpret_cast<const Symbol *>(DRI.p);
-  return Sym->Flags;
+  assert(DRI.d.a < Symbols.size());
+  return Symbols[DRI.d.a].Flags;
----------------
same here


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81717