[PATCH] D63104: Add GSYM utility files along with unit tests.

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: include/llvm/DebugInfo/GSYM/Range.h:30
+  uint64_t size() const { return Start < End ? End - Start : 0; }
+  void setStartAddress(uint64_t Addr) { Start = Addr; }
+  void setEndAddress(uint64_t Addr) { End = Addr; }
----------------
These mutable setters do not seem very useful. I deleted them in r364637


================
Comment at: include/llvm/DebugInfo/GSYM/Range.h:40
+  bool contains(uint64_t Addr) const { return Start <= Addr && Addr < End; }
+  bool doesAdjoinOrIntersect(const AddressRange &RHS) const {
+    return (Start <= RHS.End) && (End >= RHS.Start);
----------------
jakehehrlich wrote:
> maybe "isContiguousWith"
The name still doesn't make much sense to me. Deleted in r364634


================
Comment at: include/llvm/DebugInfo/GSYM/Range.h:59
+}
+inline bool operator<(const AddressRange &LHS, uint64_t Addr) {
+  return LHS.Start < Addr;
----------------
Comparison between different types of objects may bring confusion. They were only used in upper_bound. I deleted them in r364634.


================
Comment at: include/llvm/DebugInfo/GSYM/Range.h:68
+
+typedef std::vector<AddressRange> AddressRanges;
+bool contains(const AddressRanges &Ranges, uint64_t Addr);
----------------
jakehehrlich wrote:
> clayborg wrote:
> > jakehehrlich wrote:
> > > prefer using
> > llvm/include/llvm/DebugInfo/GSYM/Range.h:71:12: error: using declaration cannot refer to a template specialization
> > using std::vector<AddressRange> AddressRanges;
> >            ^     ~~~~~~~~~~~~~~
> > 
> `using AddressRanges = std::vector<AddressRange>;` is what I meant.
Fixed in r364634


================
Comment at: llvm/trunk/unittests/DebugInfo/GSYM/CMakeLists.txt:2
+set(LLVM_LINK_COMPONENTS
+  ${LLVM_TARGETS_TO_BUILD}
+  AsmPrinter
----------------
thakis wrote:
> This looks unused?
Removed in r364634


Repository:
  rL LLVM

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

https://reviews.llvm.org/D63104