[PATCH] D73086: [DWARF5] Added support for debug_macro section parsing and dumping in llvm-dwarfdump.

[Sourabh Singh Tomar via Phabricator via llvm-commits]

SouraVX marked an inline comment as done and an inline comment as not done.
SouraVX added inline comments.


================
Comment at: llvm/include/llvm/DebugInfo/DWARF/DWARFDebugMacro.h:87
+    // A value 0 in the `Header.Version` field indicates that we're parsing
+    // macinfo[.dwo] section which doesn't have header it self, hence
+    // for that case other fields in the `Header` are uninitialized.
----------------
jhenderson wrote:
> parsing a macinfo...
> it self -> itself
Is it okay to keep it the way it self or something like "`macinfo` or `macinfo.dwo` section".
Since this also covers `macinfo.dwo` case also. ?


================
Comment at: llvm/test/DebugInfo/X86/debug-macro-macinfo.s:2
+# RUN: llvm-mc -triple x86_64-unknown-linux --filetype=obj -dwarf-version=5 %s -o -|\
+# RUN: llvm-dwarfdump -v - | FileCheck %s
+
----------------
jhenderson wrote:
> jhenderson wrote:
> > You should have a verbose and non-verbose version of this test, as the printing is usually different.
> > 
> > Also, consider adding --strict-whitespace and --match-full-lines to the FileCheck call to ensure the whole content is checked and spacing is correct.
> I don't see a verbose case still?
I think we don't need one "verbose" case, since there is no difference b/w both cases.


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

https://reviews.llvm.org/D73086