[PATCH] D75375: [MCDwarf] Generate DWARF v5 .debug_rnglists for assembly files

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/lib/MC/MCDwarf.cpp:941
                              const MCSymbol *LineSectionSymbol,
-                             const MCSymbol *RangesSectionSymbol) {
+                             const MCSymbol *RangesStartSymbol) {
   MCContext &context = MCOS->getContext();
----------------
probinson wrote:
> This name change seems inappropriate.  This symbol is not paired with an End symbol, and for the GenDwarf case there is only one range list.
Rename to RangesSymbol?


================
Comment at: llvm/lib/MC/MCDwarf.cpp:1147
+      const MCSymbol *StartSymbol = Sec->getBeginSymbol();
+      const MCSymbol *EndSymbol = Sec->getEndSymbol(context);
 
----------------
probinson wrote:
> Why remove the asserts?
MCSymbolRefExpr::create asserts the non-nullness.


================
Comment at: llvm/lib/MC/MCDwarf.cpp:1186
   MCSymbol *InfoSectionSymbol = nullptr;
-  MCSymbol *RangesSectionSymbol = nullptr;
+  MCSymbol *RangesStartSymbol = nullptr;
 
----------------
probinson wrote:
> This seems like a gratuitous name change.  There is only one range list, and it has its own section, so the old name seems fine.
For .debug_rnglists, It is no longer at the start of the section, so the old name is not appropriate.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75375