Fix & enable irept pretty-printer #5031

JohnDumbell · 2019-08-15T15:28:52Z

Makes minor changes to the existing irept pretty-printer to make it work in CLion and then enable it. Marked as do-not-merge for now as I start using it locally to make sure there's no obvious problems.

Each commit message has a non-empty body, explaining why the change was made.
Methods or procedures I have added are documented, following the guidelines provided in CODING_STANDARD.md.
The feature or user visible behaviour I have added or modified has been documented in the User Guide in doc/cprover-manual/
Regression or unit tests are included, or existing tests cover the modified code (in this case I have detailed which ones those are in the commit message).
My commit message includes data points confirming performance improvements (if claimed).
My PR is restricted to a single feature or bugfix.
White-space or formatting changes outside the feature-related changed lines are in commits of their own.

thk123 · 2019-08-15T16:52:58Z

AFAICT this doesn't print the .id() for ireps that have children - I think this is a bit of a problem - can we at least have it is as a fake entry in the children:

id symbol
[0] ..
[identifier] ...

?

Will definitely give this a play though!

allredj

✔️
Passed Diffblue compatibility checks (cbmc commit: 6e96ad5).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/123400062

JohnDumbell · 2019-08-15T17:17:00Z

I'm going to make it so that it goes child: {id}, so:

0: symbol
statement: assignment

Means we can drop the fake child element, condense the tree a little and also add the missing info back in. Will make things much clearer, even if we are abusing the heck out of the pretty-printer at this stage.

NathanJPhillips

Looks good but can we see the output in plain old GDB before approving?

NathanJPhillips · 2019-08-16T08:52:55Z

scripts/pretty-printers/gdb/pretty_printers.py

+        # If we have no children it means we're actually a value node, so
+        # return the value of the node and a throw-away second value.
+        if sub_count == 0 and named_sub_count == 0:
+            yield (self.to_string(), self.val)


Why not just not return any children?

It needed this originally, doesn't anymore so has been removed.

JohnDumbell · 2019-08-16T11:28:07Z

Pushed a change that means we get a more succinct tree in CLion, like so:

(Nodes with [...] can be expanded, quoted things are values)

But as the pretty-printer is an array it makes the p command in GDB directly a little less useful, as it omits any data about the position which we use to add more context:

{<exprt> = {<irept> = "code" = {"nil", "symbol" = {"code" = {"empty", "", "1"}, "__CPROVER_initialize"}, "arguments", "empty", "nil", "function_call"}, <No data fields>}, <No data fields>}

I'll try the map quickly and see if that can work, but I really like the way it looks right now.

allredj

✔️
Passed Diffblue compatibility checks (cbmc commit: 350ec40).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/123501029

codecov-io · 2019-08-16T12:17:22Z

Codecov Report

Merging #5031 into develop will not change coverage.
The diff coverage is 100%.

@@           Coverage Diff            @@
##           develop    #5031   +/-   ##
========================================
  Coverage    69.71%   69.71%           
========================================
  Files         1322     1322           
  Lines       109429   109429           
========================================
  Hits         76287    76287           
  Misses       33142    33142

Impacted Files	Coverage Δ
jbmc/src/java_bytecode/remove_java_new.cpp	`98.47% <100%> (ø)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3ea03e4...3a344a4. Read the comment docs.

JohnDumbell · 2019-08-16T12:23:22Z

I've added the option to switch between GDB/CLion pretty-printing mode as I didn't want to sacrifice either view. Example of the CLion mode is above, this is what GDB mode looks like:

$1 = {<exprt> = {<irept> = "code" = {[0] = "nil", [1] = "symbol" = {[type] = "code" = {[return_type] = "empty", [parameters] = "", [#java_method_type] = "1"}, [identifier] = "__CPROVER_initialize"}, [2] = "arguments", [type] = "empty", [#source_location] = "nil", [statement] = "function_call"}, <No data fields>}, <No data fields>}

allredj

✔️
Passed Diffblue compatibility checks (cbmc commit: c749cd4).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/123508581

owen-mc-diffblue · 2019-08-21T11:00:19Z

scripts/pretty-printers/gdb/options.json

@@ -0,0 +1,3 @@
+{
+  "clion_pretty_printers": true
+}


Missing newline 😄

JohnDumbell · 2019-09-18T10:47:42Z

@thk123 Can I get an approval please, want to merge this soon. Owens been helping me test, and I've been able to use it personally for a few days and seems solid enough for general use.

Three things that are still on the to-do at this point:

Fix instructiont properly. This might take a while.
Make it so you don't have to expand a hierarchy to get the useful info. Shouldn't be too tricky, but it's a minor annoyance right now.
Add an ID to the root node that mimics how it works otherwise ([symbol], etc), as it's the only piece of data that's not currently shown. (Adding a fake ID node to the top node is clumsy and fiddly, adding one to every node is just bloated, all other options require some experimentation which right now I don't have time for.

thk123

Truly you are doing the Lords work. I just gave this a spin and it is amazing 😍

allredj

✔️
Passed Diffblue compatibility checks (cbmc commit: bfd2313).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/128316637

allredj

⚠️
This PR failed Diffblue compatibility checks (cbmc commit: c6f2844).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/128327640
Status will be re-evaluated on next push.
Common spurious failures include: the cbmc commit has disappeared in the mean time (e.g. in a force-push); the author is not in the list of contributors (e.g. first-time contributors); compatibility was already broken by an earlier merge.

allredj

✔️
Passed Diffblue compatibility checks (cbmc commit: 3a344a4).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/128331097

JohnDumbell added the do not merge label Aug 15, 2019

JohnDumbell requested a review from NathanJPhillips August 15, 2019 15:28

JohnDumbell requested review from peterschrammel and thk123 as code owners August 15, 2019 15:28

JohnDumbell self-assigned this Aug 15, 2019

allredj reviewed Aug 15, 2019

View reviewed changes

NathanJPhillips reviewed Aug 16, 2019

View reviewed changes

JohnDumbell force-pushed the jd/enhancement/pretty-printers branch from 6e96ad5 to 350ec40 Compare August 16, 2019 11:21

allredj reviewed Aug 16, 2019

View reviewed changes

JohnDumbell force-pushed the jd/enhancement/pretty-printers branch from 350ec40 to c749cd4 Compare August 16, 2019 12:29

allredj reviewed Aug 16, 2019

View reviewed changes

owen-mc-diffblue reviewed Aug 21, 2019

View reviewed changes

scripts/pretty-printers/gdb/options.json

@@ -0,0 +1,3 @@

{

"clion_pretty_printers": true

}

Copy link

Contributor

owen-mc-diffblue Aug 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing newline 😄

JohnDumbell force-pushed the jd/enhancement/pretty-printers branch from c749cd4 to 08b703a Compare September 17, 2019 16:07

owen-mc-diffblue approved these changes Sep 18, 2019

View reviewed changes

thk123 approved these changes Sep 19, 2019

View reviewed changes

Fix & enable irept pretty-printer

bfd2313

JohnDumbell force-pushed the jd/enhancement/pretty-printers branch from 08b703a to bfd2313 Compare September 19, 2019 09:25

allredj reviewed Sep 19, 2019

View reviewed changes

JohnDumbell requested review from cesaro, cristina-david, pkesseli and smowton as code owners September 19, 2019 10:50

Fix clang-format

3a344a4

JohnDumbell force-pushed the jd/enhancement/pretty-printers branch from c6f2844 to 3a344a4 Compare September 19, 2019 10:56

allredj reviewed Sep 19, 2019

View reviewed changes

JohnDumbell removed the do not merge label Sep 19, 2019

allredj reviewed Sep 19, 2019

View reviewed changes

JohnDumbell merged commit da454ce into diffblue:develop Sep 19, 2019

owen-mc-diffblue deleted the jd/enhancement/pretty-printers branch September 19, 2019 15:14

Fix & enable irept pretty-printer #5031

Fix & enable irept pretty-printer #5031

Uh oh!

Conversation

JohnDumbell commented Aug 15, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thk123 commented Aug 15, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

allredj left a comment

Choose a reason for hiding this comment

Uh oh!

JohnDumbell commented Aug 15, 2019

Uh oh!

NathanJPhillips left a comment

Choose a reason for hiding this comment

Uh oh!

NathanJPhillips Aug 16, 2019

Choose a reason for hiding this comment

Uh oh!

JohnDumbell Aug 16, 2019

Choose a reason for hiding this comment

Uh oh!

JohnDumbell commented Aug 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

allredj left a comment

Choose a reason for hiding this comment

Uh oh!

codecov-io commented Aug 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

JohnDumbell commented Aug 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

allredj left a comment

Choose a reason for hiding this comment

Uh oh!

owen-mc-diffblue Aug 21, 2019

Choose a reason for hiding this comment

Uh oh!

JohnDumbell commented Sep 18, 2019

Uh oh!

thk123 left a comment

Choose a reason for hiding this comment

Uh oh!

allredj left a comment

Choose a reason for hiding this comment

Uh oh!

allredj left a comment

Choose a reason for hiding this comment

Uh oh!

allredj left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

JohnDumbell commented Aug 15, 2019 •

edited

Loading

thk123 commented Aug 15, 2019 •

edited

Loading

JohnDumbell commented Aug 16, 2019 •

edited

Loading

codecov-io commented Aug 16, 2019 •

edited

Loading

JohnDumbell commented Aug 16, 2019 •

edited

Loading