This is an archive of the discontinued LLVM Phabricator instance.

Add pretty-printer for llvm::Twine type
ClosedPublic

Authored by simark on Mar 15 2017, 12:00 PM.

Details

Summary

This patch adds a basic pretty-printer for llvm::Twine.

Diff Detail

Repository
rL LLVM

Event Timeline

simark created this revision.Mar 15 2017, 12:00 PM

I'm not very familiar with gdb pretty printers but I tested the patch and it works well! This will definitely help debugging.

dblaikie added inline comments.Mar 15 2017, 12:39 PM
utils/gdb-scripts/prettyprinters.py
205–219

This seems more complicated than I would've expected, is str(val) insufficient?

224–265

Rather than a long else-if chain with various assignments to 's', perhaps a more LLVM-esque style would be to return from each condition:

if kind == ...:
  return str(...)
if kind == UHexKind:
  return hex(int(val))
/* do the unsupported NodeKind thing here */
simark marked an inline comment as done.Mar 15 2017, 1:16 PM
simark added inline comments.
utils/gdb-scripts/prettyprinters.py
205–219

That's what I did first. Actually, it was s = str(val)[1:-1], because the return of str(val) contains quotes.

But I wanted to give a better experience (a good error message) when the std::string pretty printer is not loaded (which can happen easily, as they're a bit of a pain to set up sometimes). Otherwise, you get something like this as part of the resulting string:

"foostatic npos = <optimized out>, _M_dataplus = {<std::allocator<char>> = {<__gnu_cxx::new_allocator<char>> = {<No data fields>}, <No data fields>}, _M_p = 0x318e1f8 \"bar\"}baz"

Instead, the goal is to print a warning:

(gdb) print twine
$1 = No pretty printer for std::string found. The resulting Twine representation will be incomplete.
"foo<std::string>baz"

What's broken is the

self.string_from_pretty_printer_lookup(val).value().address.string()

I do lower. I added that because the std::string pretty printer returns a gdb.LazyString, and that's the only way I've found to convert it to a Python string. However, it doesn't work when we return the placeholder <std::string>.

The other thing is that a <std::string> placeholder looks very much like a C++ template, so it's probably not a very good choice. I wanted to add something to the string to make the user realize that something should be there but is missing, but that format probably looks too much like some "valid" content.

simark updated this revision to Diff 91919.Mar 15 2017, 1:17 PM
  • Add return statements in the if-else chain
  • Fix case when std::string pretty printer is not loaded
simark added inline comments.Mar 15 2017, 1:19 PM
utils/gdb-scripts/prettyprinters.py
217

This is really ugly, but I didn't find any other way to check if the return is of that type. GDB doesn't seem to register the LazyString type:

>>> gdb.Value
<type 'gdb.Value'>
>>> gdb.LazyString
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: 'module' object has no attribute 'LazyString'

so I can't do type(s) == gdb.LazyString...

dblaikie accepted this revision.Mar 15 2017, 1:30 PM

Looks good (one minor thing to fix up further - the elifs) & thanks for explaining the stringification issue. Maybe add some comments there about why it's needed/useful, if you like (& why the type comparison is difficult/awkward)

utils/gdb-scripts/prettyprinters.py
232–264

Remove the "el"/elses here - since each condition unconditionally returns:

if ...:
  return
if ...:
  return
return '(unhandled...
This revision is now accepted and ready to land.Mar 15 2017, 1:30 PM
simark updated this revision to Diff 91926.Mar 15 2017, 1:48 PM
simark marked an inline comment as done.

Formatting and comments.

This revision was automatically updated to reflect the committed changes.