[Lldb-commits] [PATCH] D122943: [LLDB][NativePDB] Fix a crash when S_DEFRANGE_SUBFIELD_REGISTER descirbes a simple type

[Zequan Wu via Phabricator via lldb-commits]

zequanwu added inline comments.


================
Comment at: lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp:54
 struct FindMembersSize : public TypeVisitorCallbacks {
-  FindMembersSize(std::vector<std::pair<RegisterId, uint32_t>> &members_info,
+  FindMembersSize(llvm::SmallVector<std::pair<RegisterId, uint32_t>> &members_info,
                   TpiStream &tpi)
----------------
aganea wrote:
> labath wrote:
> > I think `SmallVectorImpl` is still the official way to take SmallVector references.
> `ArrayRef` would be even better! :) ie. `llvm::ArrayRef<std::pair<RegisterId, uint32_t>> members_info;`
Using `SmallVectorImpl` because we want to insert elements in `visitKnownMember`.


================
Comment at: lldb/test/Shell/SymbolFile/NativePDB/subfield_register_simple_type.s:27
+# CHECK:      LineEntry: [0x00401026-0x00401039): C:\src\a.cpp:3
+# CHECK-NEXT: Variable: id = {{.*}}, name = "x", type = "int64_t", valid ranges = [0x0040102f-0x00401036), location = DW_OP_reg0 EAX, DW_OP_piece 0x4, DW_OP_reg2 EDX, DW_OP_piece 0x4, decl =
+
----------------
aganea wrote:
> nitpick: Is anything after (and including) ", valid ranges" necessary to cover this change?
Yes, the structure of `location` is basically what we are testing against though the register number maybe incorrect due to the `FIXME` above.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122943