[PATCH] D89845: Add the ability to extract the unwind rows from DWARF Call Frame Information.

[George Rimar via Phabricator via llvm-commits]

grimar added a comment.

Few minor nits from me (I am not an expert to review this).



================
Comment at: llvm/include/llvm/DebugInfo/DWARF/DWARFDebugFrame.h:259
+  uint64_t getAddress() const {
+    assert(Address.hasValue());
+    return *Address;
----------------
No need to have an assert to check `hasValue` here and below,
the `Optional<>` does it internally:


```
  constexpr T const &getValue() const LLVM_LVALUE_FUNCTION noexcept {
    assert(hasVal);
    return value;
  }
```


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFDebugFrame.cpp:1003
+  else
+    OS << "  error: decoding the CIE opcodes  into rows failed: "
+       << toString(RowsOrErr.takeError()) << "\n";
----------------
Has an excessive double space after "opcodes".


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFExpression.cpp:508
+  if (Data.getData().size() != RHS.Data.getData().size())
+    return false;
+  return Data.getData() == RHS.Data.getData();
----------------
You can remove this check, `StringRef` does length comparsion internally:


```
    bool equals(StringRef RHS) const {
      return (Length == RHS.Length &&
              compareMemory(Data, RHS.Data, RHS.Length) == 0);
    }

```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89845