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

[Adrian Prantl via Phabricator via llvm-commits]

aprantl added inline comments.


================
Comment at: lib/CodeGen/AsmPrinter/DebugHandlerBase.cpp:170
 
+/// Collect labels in this machine function into DebugHandlerBase::DbgLabels.
+///
----------------
This comment should go to the declaration.


================
Comment at: lib/CodeGen/AsmPrinter/DebugHandlerBase.h:53
+using InlinedLabel =
+      std::pair<const DILabel *, const DILocation *>;
+using LabelInstrMap = MapVector<InlinedLabel, const MachineInstr *>;
----------------
HsiangKai wrote:
> aprantl wrote:
> > Often it's better for readability to just define a struct with named members so we can access  .label and .loc instead of .first and .second.
> If I group DILabel and DILocation in a struct, I need to add isEqual(), getHashValue(), getEmptyKey(), etc to conform the interface of MapVector. I think it is too complex for the purpose. std::pair is ready to use as the 'Key Type' of MapVector.
> 
> Use MapVector is easier to establish the mapping between InlinedLabel and MachineInstr.
Fair point. One trick I've used in the past for situations like this is to define a struct that inherits from std::pair<> and defines a getLabel() and a getLocation() accessor function. Feel free to adopt this pattern if you think that it improves readability.


================
Comment at: lib/CodeGen/AsmPrinter/DwarfDebug.h:191
+/// Labels are collected from \c DBG_LABEL instructions.
+class DwarfLabel {
+  const DILabel *Label;                 /// Label Descriptor.
----------------
aprantl wrote:
> Could this share a common base with DbgVariable?
Is this feasible?


Repository:
  rL LLVM

https://reviews.llvm.org/D45556