[PATCH] D136249: [NFC][SelectionDAG][DebugInfo] Refactor DanglingDebugInfo class

[Stephen Tozer via Phabricator via llvm-commits]

StephenTozer added a comment.

A few minor inline comments, other than that LGTM.



================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.h:111
     DanglingDebugInfo() = default;
     DanglingDebugInfo(const DbgValueInst *di, DebugLoc DL, unsigned SDNO)
+        : DI(di), SDNodeOrder(SDNO) {}
----------------
DL can be removed as an argument now, right? Also just as a point of style, could you drive-by change the first argument to use correct casing?


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.h:117
+    Value *getVariableLocationOp(unsigned Idx) const {
+      assert(Idx == 0 && "variadic not supported yet");
+      return DI->getVariableLocationOp(0);
----------------
Would request the assert text be updated with a little more detail - also if there will be an assert here, it's probably a good idea to put one in the constructor too.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.h:120
+    }
+    DebugLoc getdl() const { return DI->getDebugLoc(); }
+    unsigned getSDNodeOrder() const { return SDNodeOrder; }
----------------
Apologies since this will probably increase the line-count of this change, but while we're refactoring this interface could we just rename this? The original name is unnecessarily terse and doesn't follow the style guidelines, and now that it doesn't even match the name of a member it should probably be changed.


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

https://reviews.llvm.org/D136249