[PATCH] D78920: [llvm-readobj] [COFF] Cope with debug directory payloads in unmapped areas

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 29 01:34:30 PDT 2020


jhenderson accepted this revision.
jhenderson added a comment.
This revision is now accepted and ready to land.

LGTM, with the suggestions.



================
Comment at: llvm/test/tools/llvm-readobj/COFF/debug-directory-unmapped.test:1-4
+# Test that printing debug directories that aren't part of the runtime
+# mapped sections doesn't fail. (Currently only printing the entry itself
+# but not the payload - and this test doesn't currently include any
+# meaningful data where it claims that the debug entry payload is.)
----------------
Could you use '##' for the comment markers, please. This is a practice I'm encouraging in my reviews, as it helps comments stand out from the lit/FileCheck directives.

Also I'd change the bit in brackets to "Currently llvm-readobj only prints the entry itself and not the payload. Note that there isn't currently any meaningful data in this test input where it claims the debug entry payload is." and probably just get rid of the brackets.


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

https://reviews.llvm.org/D78920





More information about the llvm-commits mailing list