[PATCH] D44772: [DWARF] Simplify DWARFAddressRange::{intersects, contains}.

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: include/llvm/DebugInfo/DWARF/DWARFAddressRange.h:39
   bool intersects(const DWARFAddressRange &RHS) const {
-    // Empty ranges can't intersect.
-    if (LowPC == HighPC || RHS.LowPC == RHS.HighPC)
-      return false;
-    return (LowPC < RHS.HighPC) && (HighPC > RHS.LowPC);
+    return LowPC < RHS.HighPC && RHS.LowPC < HighPC;
   }
----------------
MaskRay wrote:
> jhenderson wrote:
> > There's a change in behaviour here, which I actually agree with, although I wonder whether it could have a negative impact on the clients. Given a range [N, N+2), I would expect an empty range of [N+1, N+1) to be classified as intersecting with it. Empty ranges at either end of the non-empty range are less clear-cut though, and I don't think should count as intersecting.
> Added an assert as the call site has checked the validity.
I might have missed something, but won't this assert() fire on some of the unit tests? If we think they're not really valid, please could you delete the relevant ones.


Repository:
  rL LLVM

https://reviews.llvm.org/D44772