[PATCH] D72828: [DWARF5] Added support for emission of debug_macro section.

[Igor Kudrin via Phabricator via llvm-commits]

ikudrin requested changes to this revision.
ikudrin added inline comments.
This revision now requires changes to proceed.


================
Comment at: llvm/include/llvm/BinaryFormat/Dwarf.def:830
+// DWARF v5 Macro header flags
+HANDLE_MACRO_FLAGS(0x00, OFFSET_SIZE_32)
+HANDLE_MACRO_FLAGS(0x01, OFFSET_SIZE_64)
----------------
SouraVX wrote:
> ikudrin wrote:
> > Remove `OFFSET_SIZE_32`, as this value does not define any real flag.
> Yes, theirs no particular flag in Spec. Spec says about "offset_size_flag" and how it's value is interpreted
> 0 - DWARF32 1 - DWARF64. I created these flags for readable/informative purpose.
This is false readability. You still have to remember that the value is not real and treat it accordingly. There is no distinct value for DWARF32 in the specs. The value of 0 means that the contribution has DWARF32 format AND does not have a debug_line_offset field AND does not have an opcode_operands_table. Moreover, you do not define pairs of values for the other flags, which makes all the definition inconsistent. So, please, remove that fictitious value and make the set of flags follow the documentation.


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

https://reviews.llvm.org/D72828