[PATCH] Fix DWARFContext::getCompileUnitForOffset().

[David Blaikie]

================
Comment at: lib/DebugInfo/DWARFUnit.h:50
@@ -57,2 +49,3 @@
     bool operator()(uint32_t LHS,
                     const std::unique_ptr<UnitType> &RHS) const {
+      return LHS < RHS->getNextUnitOffset();
----------------
friss wrote:
> dblaikie wrote:
> > I'm rather surprised if this is the one function that's required. According to the spec, it looks like the specified value is passed as the RHS, not the LHS... 
> Note I'm using upper_bound bellow. The link you provided says:
> Change 25.3.3.2 [lib.upper.bound] paragraph 1 from:
> -1- Requires: Type T is LessThanComparable (lib.lessthancomparable).
> to:
> -1- Requires: The elements e of [first, last) are partitioned with respect to the expression !(value < e) or !comp(value, e)
> 
> So value is on the LHS and the element at the RHS.
Oh, right - thanks for the explanation.

In that case, could we write this comparison as "LHS < RHS->getOffset()". That would be the real less-than comparison of the offset.

But then I suppose you'd have to switch around to lower_bound? Would there be a problem with that? 

Sorry I'm being rather confused by all this... 

It's just given the definition of upper_bound, it's sort of weird to give a less than comparison that's subtly different (doesn't actually compare !< for the same unit) and then access the element returned - when the element should be one /past/ the equal elements you're looking for... except for the subtle comparison which ends up doing what you want.

So I suppose what I'm trying to suggest is:

  operator()(unique_ptr LHS, unsigned RHS)
    return LHS->getNextUnitOffset() <= RHS; // this is actually a "less than" comparison, because "next unit offset" is past the end of this unit, this could alternatively be written as "getNextUnitOffset() - 1 < RHS"
  }
  ...
  auto I = lower_bound... 
  ...

http://reviews.llvm.org/D5262