mib marked 2 inline comments as done. mib added inline comments.
================ Comment at: lldb/include/lldb/Symbol/LineTable.h:344 + template <typename T> + uint32_t FindLineEntryIndexByFileIndexImpl( + uint32_t start_idx, T file_idx, ---------------- JDevlieghere wrote: > It seems like this method is only using `m_entries`, so I would turn this > into a static (non-member) function in the cpp file and pass `m_entries` as > an argument. Besides m_entries, the function also calls `LineTable::ConvertEntryAtIndexToLineEntry` which is declared `protected`. Making the function "static" makes it more convoluted, IMO. I'd rather leave it in the header file. ================ Comment at: lldb/include/lldb/Symbol/LineTable.h:377 + continue; + } else if (m_entries[idx].line == line) { + ConvertEntryAtIndexToLineEntry(idx, *line_entry_ptr); ---------------- JDevlieghere wrote: > I know you're just moving the code but maybe a good opportunity to simplify > the else-after-continue and else-after-return below. As discussed, I'll simplify these in a follow-up patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101221/new/ https://reviews.llvm.org/D101221 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits