irep_idt pretty printer improvements #4942
Conversation
| except: | ||
| return "" | ||
| return "Exception evaluating dstringt" |
There was a problem hiding this comment.
I don't think showing the exception is useful to anyone unless they're debugging the pretty printers, better to just leave it blank so it doesn't show anything at all.
There was a problem hiding this comment.
You might be right. Though a blank string may be confusing to the user?
There was a problem hiding this comment.
When I tested it before it wasn't a blank string but no value to be printed, might've changed a little with the type also changing, but that'd be the best result.
There was a problem hiding this comment.
OK, dropped that commit
| imports.add(lines[line_no][len("import "):].strip()) | ||
| lines.pop(line_no) | ||
| else: | ||
| if lines[line_no].startswith('cbmc_printers_folder = '): |
There was a problem hiding this comment.
Pull the string out into a variable if you're using it for an anchor to detect the first line:
script_start_anchor = "cbmc_printers_folder = "
| lines = [ line.rstrip() for line in file ] | ||
| line_no = 0 | ||
| imports = { "os", "sys" } | ||
| while line_no < len(lines): |
There was a problem hiding this comment.
I was reluctant to put this in originally as if you had a .gdbinit file you already knew enough to apply this directly. You might want to save a backup.gdbinit somewhere just in case things are detected incorrectly and you make unwanted modifications to the file that can't be got back.
| return "<Optimized out>" | ||
| return value | ||
| return "{}: <Optimized out>".format(string_no) | ||
| return "{}: \"{}\"".format(string_no, value.string().replace("\0", "").replace("\"", "\\\"")) |
There was a problem hiding this comment.
I'm not convinced showing the number has any value when you have the string representation, for all intents and purposes it's just a string, no point confusing people new to it.
There was a problem hiding this comment.
✔️
Passed Diffblue compatibility checks (cbmc commit: 0b1648f).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/120253808
Codecov Report
@@ Coverage Diff @@
## develop #4942 +/- ##
===========================================
- Coverage 69.26% 69.26% -0.01%
===========================================
Files 1307 1307
Lines 108089 108087 -2
===========================================
- Hits 74868 74866 -2
Misses 33221 33221
Continue to review full report at Codecov.
|
|
@JohnDumbell isn't sure displaying the dstringt number is helpful in which case I can drop that commit. Does anyone else have any opinions in either direction? |
Make it: * able to detect when it has already been run * able to add script to existing file without breaking existing printers * add new script before existing printers that might clash with it (such as the C++ ones) * executable
Adjust the display_hint type and the formatting to show string as before except after the number
NathanJPhillips
force-pushed
the
feature/irep_idt_pretty
branch
from
0b1648f to
2eca2b1
Compare
There was a problem hiding this comment.
✔️
Passed Diffblue compatibility checks (cbmc commit: 2eca2b1).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/120505314
|
I don't think showing the number would be confusing (practice will show whether it's useful, though). |
This updates the irep_idt pretty printers to include the dstring number and to cope better with uninitialised dstrings or optimised-out values. It also makes the installation script more resilient for people who already have the libstd pretty printers installed.