[PATCH] D45556: [DebugInfo] Generate DWARF debug information for labels.

[Adrian Prantl via Phabricator via llvm-commits]

aprantl added a comment.

I think this is starting to look good. I have a few more comments inline.



================
Comment at: lib/CodeGen/AsmPrinter/DbgEntityHistoryCalculator.h:57
 
-void calculateDbgValueHistory(const MachineFunction *MF,
-                              const TargetRegisterInfo *TRI,
-                              DbgValueHistoryMap &Result);
+class DbgLabelInstrMap {
+public:
----------------
Please add a Doxygen comment explaining what this class is used for.


================
Comment at: lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp:829
+
+  auto *Die = Entity->getDIE();
+  if (AbsEntity && AbsEntity->getDIE())
----------------
The old code on the left had more comments :-)


================
Comment at: lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp:835
+      applyVariableAttributes(*Var, *Die);
+    else if (const DbgLabel *Label = dyn_cast<const DbgLabel>(Entity))
+      applyLabelAttributes(*Label, *Die);
----------------
can this be anything else? Should this be a static cast instead?


================
Comment at: lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp:839
 
-DbgVariable *DwarfCompileUnit::getExistingAbstractVariable(InlinedVariable IV) {
-  const DILocalVariable *Cleansed;
-  return getExistingAbstractVariable(IV, Cleansed);
+  /* TODO
+  if (Label) {
----------------
Please don't check in commented-out code.


================
Comment at: lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp:857
+                                            LexicalScope *Scope) {
   assert(Scope && Scope->isAbstractScope());
+  if (isa<const DILocalVariable>(Node)) {
----------------
I think you may be able to write this more compactly by
```
auto &MapEntry = getAbstractEntities()[Node];
if (...) {
  MapEntry = llvm>::make_unique(...)
  DU->addScopeVariable(Scope, cast<DbgVariable>(MapEntry.get());
} else
```


================
Comment at: lib/CodeGen/AsmPrinter/DwarfDebug.h:64
 
+class DbgEntity {
+  const DINode *Entity;
----------------
Doxygen comment?


================
Comment at: lib/CodeGen/AsmPrinter/DwarfDebug.h:79
+
+  // Accessors.
+  const DINode *getEntity() const { return Entity; }
----------------
You can define group comments like this:
```
/// Accessors
/// @{
...
/// @}
```


Repository:
  rL LLVM

https://reviews.llvm.org/D45556