[PATCH] D32713: [DWARFv5] Parse new line-table header format.

[Paul Robinson via Phabricator via llvm-commits]

probinson marked 12 inline comments as done.
probinson added a comment.

I sent DWARFDebugLine to the stylist in https://reviews.llvm.org/rL301883.  Review for StringRef-ized paths in https://reviews.llvm.org/D32728.  Other requested changes in-plan where indicated.
Likely more as well but it's time to go home.



================
Comment at: lib/DebugInfo/DWARF/DWARFDebugLine.cpp:159-160
+  }
+  if (IncludeDirectories.size() != EntryCount)
+    return false;
+
----------------
dblaikie wrote:
> Looks like this only happens if Descriptors doesn't contain DW_LNCT_path - worth checking for that separately/up-front to be more explicit?
I can have parseV5EntryFormat check that DW_LNCT_path is present, then the only failure mode is if extractValue or skipValue returns false, and this check becomes unnecessary.


================
Comment at: test/DebugInfo/Inputs/dwarfdump-header.s:205
+        .short  5               # DWARF version number
+	.byte   8               # Address Size
+        .byte   0               # Segment Selector Size
----------------
dblaikie wrote:
> indent?
I think Emacs snuck a tab in there on me.


================
Comment at: test/DebugInfo/Inputs/dwarfdump-header.s:231
+        .byte   0x08            # DW_FORM_string
+	# Directory table entries
+        .byte   2               # Two directories
----------------
dblaikie wrote:
> indent?
Tab.


https://reviews.llvm.org/D32713