[Lldb-commits] [lldb] [lldb] Fix the semantics of SupportFile equivalence (PR #95606)
Jason Molenda via lldb-commits
lldb-commits at lists.llvm.org
Mon Jun 17 22:37:02 PDT 2024
jasonmolenda wrote:
> > Thanks for the overwhelming support :-)
>
> I hate to ruin a party, but I don't think this is a good use of `operator==`, precisely because it does not define an equivalence relation (it's not transitive). Might I suggest named function instead? `IsCompatible` ?
The place I hit a problem with this was in `ThreadPlanStepRange::InRange` where we're doing
```
if (m_addr_context.line_entry.IsValid() &&
new_context.line_entry.IsValid()) {
if (*m_addr_context.line_entry.original_file_sp ==
*new_context.line_entry.original_file_sp) {
if (m_addr_context.line_entry.line == new_context.line_entry.line) {
m_addr_context = new_context;
const bool include_inlined_functions =
GetKind() == eKindStepOverRange;
...
```
and only one of the SupportFile's here had a checksum, so the == test failed (and an inlined stepping bug came up)
If I read `m_addr_context.line_entry.original_file_sp->IsCompatible(new_context.line_entry.original_file_sp)` I would think "Oh is this considering maybe a remapped filename?" or something, I'm not sure I would see that as "the same file, ignoring checksum if only one has it".
There are probbly some other uses of this operator== and maybe I shouldn't focus on this one, but I'm trying to think of how the intention could be clearly expressed with a method name here.
https://github.com/llvm/llvm-project/pull/95606
More information about the lldb-commits
mailing list