[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