[PATCH] D69999: [DebugInfo] Support for DW_OP_implicit_pointer (IR Verifier and Bitcode)

[Orlando Cazalet-Hyams via Phabricator via llvm-commits]

Orlando added a comment.

Hi @alok,

Adrian said:

> Thanks! Let's cycle back to this after the llvm-dev thread that @dblaikie started has been resolved.

Reading the thread I'm not sure if it counts as having been 'resolved', so hopefully these comments are not out of place. 
To help keep things running smoothly please make sure:
a) variable names are capitalised, and
b) comments are written as sentences (capitals, punctuation),



================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h:77
+  // debugging information entry that describes the dereferenced object’s value.
+  // when this operation is created, the debugging information entry is not yet
+  // formed. To solve this problem, we maintain a vector of dereferenced
----------------
when -> When


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h:79-80
+  // formed. To solve this problem, we maintain a vector of dereferenced
+  // objects. We keep index of dereferenced object's at first operand of
+  // DW_OP_implicit_pointer operation temporarily. This temporary value is
+  // later replaced by actual value when available.
----------------
I get what this is saying but I found it a little hard to parse, and as Adrian pointed out elsewhere 'later' is a bit vague:

`We keep index of dereferenced object's at first operand of DW_OP_implicit_pointer operation temporarily. This temporary value is later replaced by actual value when available.`


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h:82
+  // later replaced by actual value when available.
+  std::vector<const DINode *> ImplicitVars;
+
----------------
Could `std::vector` be `llvm::SmallVector` [0] here?

[0] http://llvm.org/docs/ProgrammersManual.html#llvm-adt-smallvector-h



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

https://reviews.llvm.org/D69999