yinghuitan added a comment.

@clayborg, now I see you want to make this option true by default while full 
paths one to be optional. Then sure, it makes sense.



================
Comment at: lldb/source/Breakpoint/BreakpointResolverFileLine.cpp:225
+  const bool case_sensitive = request_file.IsCaseSensitive();
+  const bool full = !request_file.GetDirectory().IsEmpty();
+  for (uint32_t i = 0; i < sc_list.GetSize(); ++i) {
----------------
clayborg wrote:
> If the directory is empty on the requested file, should we be doing anything 
> here? Can we early return?
This should be handled by check at line 221, right? If 
`request_file.GetDirectory().IsEmpty()` then `request_file.IsRelative()` should 
be true and early return already?


================
Comment at: lldb/source/Target/Target.cpp:405
   BreakpointResolverSP resolver_sp(new BreakpointResolverFileLine(
-      nullptr, offset, skip_prologue, location_spec));
+      nullptr, offset, skip_prologue, location_spec, removed_prefix_opt));
   return CreateBreakpoint(filter_sp, resolver_sp, internal, hardware, true);
----------------
clayborg wrote:
> The breakpoint is initialized with NULL here. Does it get set to something 
> valid before we try to use it somehow? I am worried we won't be able to get a 
> target from the BreakpointResolver's stored breakpoint????
It is initialized as nullptr here but `CreateBreakpoint()` call in next line 
will call `BreakpointResolver::SetBreakpoint()` to initialize it. Actually in 
`BreakpointResolver::GetBreakpoint()` explicitly has an assertion to ensure it 
can't be nullptr so I assume we should be safe here. 



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133042/new/

https://reviews.llvm.org/D133042

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

Reply via email to