[Lldb-commits] [PATCH] D100965: [lldb] Refactor argument group by SourceLocationSpec (NFCI)

[Jonas Devlieghere via Phabricator via lldb-commits]

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