[PATCH] D83050: [DebugInfo] Add more checks to parsing .debug_pub* sections.

[James Henderson via Phabricator via llvm-commits]

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

LGTM, I think, but please wait for @dblaikie.



================
Comment at: llvm/test/tools/llvm-dwarfdump/X86/debug_pub_tables_error_cases.s:3
+
+## All four public name sections share the same parser, but slightly different
+## code paths are used to reach it. Do a comprehensive check for one of the
----------------
ikudrin wrote:
> jhenderson wrote:
> > I think this is the first time I've heard the term "public name sections" being used. Is this called that in the standard? Otherwise, I might suggest using a different phrasing (though don't necessarily know what).
> Well, the standard sometimes uses the term "name lookup tables". Do you think now the comment sound better?
Looks okay to me.


================
Comment at: llvm/test/tools/llvm-dwarfdump/X86/debug_pub_tables_error_cases.s:7-8
+
+# RUN: not llvm-dwarfdump -debug-gnu-pubnames %t 2> %t.err | FileCheck %s
+# RUN: FileCheck %s --input-file=%t.err --check-prefix=ERR
+
----------------
ikudrin wrote:
> jhenderson wrote:
> > I don't mind too much either way, especially given the difficulties I recently had with the debug line equivalent test, but is there a particular reason you've kept the two streams separate? By combining them you can show the relative position of output for the common case of the streams being combined.
> That is done to improve readability. The error messages are printed during parsing and dumping of all sets in the section comes after that. Thus, if we want to check all the messages at once, the error messages (or dumping messages) have to be separated from the corresponding lines of source code.
Thanks, makes sense.


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

https://reviews.llvm.org/D83050