labath added a comment.

The general idea makes sense to me, although I haven't tried to understand the 
pdb parsing code.

> (otherwise internal binary search may fail)

The current search algorithm is buggy, but I think the `RangeDataVector` class 
has always intended to support overlapping ranges. If you find yourself needing 
to do extra work to work its limitations, we should fix that algorithm instead. 
(However, I know that the generic class will not be able to express the 
preference for expressions that describe the full value vs. those that describe 
only a part of it, so I'll believe you if you say that some outside 
filtering/deduplication is still needed.)



================
Comment at: lldb/source/Expression/DWARFExpressionList.cpp:37-39
+  if (m_exprs.FindEntryThatContains(base) ||
+      m_exprs.FindEntryThatContains(end - 1))
+    return false;
----------------
What is this attempting to check? That the list does not contain an overlapping 
entry? That hardly seems like a correct algorithm...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130796

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

Reply via email to