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

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to