[PATCH] D118774: [DebugInfo][InstrRef][NFC] Use unique_ptr instead of raw pointers for value tables

[Orlando Cazalet-Hyams via Phabricator via llvm-commits]

Orlando added a comment.

Nice. I have a few questions / nits but I'm a fan of the change.



================
Comment at: llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp:3144-3147
     for (int Idx = 0; Idx < MaxNumBlocks; ++Idx) {
-      delete[] MOutLocs[Idx];
-      delete[] MInLocs[Idx];
+      MOutLocs[Idx].reset();
+      MInLocs[Idx].reset();
     }
----------------
I don't think you need this explicit clean-up on this code path any longer, since the `unique_ptr` dtor will handle it for you?


================
Comment at: llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp:3165
     // locations.
     emitLocations(MF, SavedLiveIns, MOutLocs, MInLocs, AllVarsNumbering, *TPC);
 
----------------
If I'm reading this correctly, `emitLocations` is becomes responsible for managing the object lifetimes (it `delete`s things from `MOutLocs`  and `MInLocs`); should you `std::move` `MOutLocs` and `MInLocs` to signify this?


================
Comment at: llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.cpp:3172-3173
   // Elements of these arrays will be deleted by emitLocations.
-  delete[] MOutLocs;
-  delete[] MInLocs;
+  MOutLocs.reset();
+  MInLocs.reset();
 
----------------
You don't need to call `reset` since `MOutLocs` is scoped to this function (the `unqiue_ptr`'s dtor will clean up for you).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118774