wallace added inline comments.
================ Comment at: lldb/include/lldb/Target/TraceHTR.h:31 +public: + HTRBlockMetadata(Thread &thread, TraceInstruction curr_instruction, + llvm::Optional<TraceInstruction> next_instruction); ---------------- const TraceInstruction &curr_instruction to avoid making an unnecessary copy ================ Comment at: lldb/include/lldb/Target/TraceHTR.h:33 + llvm::Optional<TraceInstruction> next_instruction); + HTRBlockMetadata(size_t num_instructions, + std::unordered_map<std::string, size_t> func_calls) ---------------- pass the map by reference ================ Comment at: lldb/include/lldb/Target/TraceHTR.h:42-68 + friend llvm::json::Value toJSON(const HTRBlockMetadata &metadata); + template <class M> friend llvm::json::Value toJSON(const TraceHTR<M> &layer); +}; + +/// \class HTRBlock TraceHTR.h "lldb/Target/TraceHTR.h" +/// Trace agnostic block structure +/// Sequences of blocks are merged to create a new, single block ---------------- the consistent style is that toJSON methods are not class members but static functions ================ Comment at: lldb/include/lldb/Target/TraceHTR.h:73 +/// Maps the unique Block IDs to their respective Blocks +template <class TMetadata> class HTRBlockDefs { +public: ---------------- clayborg wrote: > Does this need to be templatized? Are we using this same class with multiple > different TMetadata classes or are we always using it with HTRBlockMetadata? +1 I don't think you should go with templates unless you had more TMetadata objects Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105741/new/ https://reviews.llvm.org/D105741 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits