[Lldb-commits] [PATCH] D122293: [wip][intelpt] Refactoring instruction decoding for flexibility

[Alisamar Husain via Phabricator via lldb-commits]

zrthxn added inline comments.


================
Comment at: lldb/source/Plugins/Trace/intel-pt/DecodedThread.h:152
+  ///   The errors of the trace.
+  std::unordered_map<uint64_t, llvm::Error> GetErrors() const;
+
----------------
jj10306 wrote:
> Return a reference here to avoid potential expensive copy when returning.
> 
> Something else to consider is if we need/want an API exposing all the entire error map or if something like:
> `llvm::Error GetErrorByInstructionIndex(uint64_t insn_index) const` that allows the caller to specify the key into the map would make more sense? 
> This also has the advantage that it hides the implementation detail of what data type is being used under the hood to represent the error map!
> @wallace @zrthxn wdyt?
Yea I did think about that. In my opinion returning a reference would be good since with that you can get the size and error at each index and we don't need to have functions for each operation which would have long clunky names making the code less readable...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122293