JDevlieghere added a comment. A few nits but overall this looks like a great cleanup!
================ Comment at: lldb/source/Breakpoint/BreakpointResolverFileLine.cpp:255 - SetSCMatchesByLine(filter, sc_list, m_skip_prologue, s.GetString(), - m_line_number, m_column); + SetSCMatchesByLine(filter, sc_list, m_skip_prologue, s.GetString(), line, + column); ---------------- Would it make sense for the column here to be passed as an optional. Right now you're unpacking it which means we potentially have to check for LLDB_INVALID_COLUMN again in the function that's called here. ================ Comment at: lldb/source/Core/Module.cpp:603-605 + const bool exact_match = false; + SourceLocationSpec location_spec(file_spec, line, /*column=*/llvm::None, + check_inlines, exact_match); ---------------- nit: This probably won't matter after the todo, but we should be consistent between using the comment or a named variable. Same observation below. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100965/new/ https://reviews.llvm.org/D100965 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
