Skip to content

Conversation

smowton
Copy link
Contributor

@smowton smowton commented Oct 19, 2018

Add --list-symbols argument to JBMC (matching existing argument in CBMC), and fix the accidental inclusion of the symbol value in the JSON output even in list mode.

  • 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 ODING_STANDARD.md.
  • 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.

Copy link
Member

@peterschrammel peterschrammel left a comment

Choose a reason for hiding this comment

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

OPT_SHOW_CLASS_HIERARCHY \
"(show-loops)" \
"(show-symbol-table)(show-parse-tree)" \
"(show-symbol-table)(list-symbols)(show-parse-tree)" \
Copy link
Member

Choose a reason for hiding this comment

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

Preferably put on separate lines.

Copy link
Collaborator

@tautschnig tautschnig left a comment

Choose a reason for hiding this comment

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

I am struggling to reconcile the commit subject with the actual changes. The notes in the PR make more sense, but seemingly those haven't made it into the commit message.

The plain-text mode correctly prints only names and types; with --json-ui until now
we accidentally printed the symbol's value as well, which should only happen with --show-symbol-table.
@smowton smowton force-pushed the smowton/feature/jbmc-list-symbols branch from 8cd5034 to 1891dcf Compare October 22, 2018 08:38
@smowton
Copy link
Contributor Author

smowton commented Oct 22, 2018

@tautschnig oops, was from a subtree cherry-pick. Fixed.

@smowton smowton force-pushed the smowton/feature/jbmc-list-symbols branch from 1891dcf to d61ad5b Compare October 22, 2018 08:59
@smowton smowton force-pushed the smowton/feature/jbmc-list-symbols branch from d61ad5b to 74c9ff9 Compare October 22, 2018 16:03
Copy link
Contributor

@allredj allredj left a comment

Choose a reason for hiding this comment

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

Passed Diffblue compatibility checks (cbmc commit: 74c9ff9).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/88760621

This mirrors the support already present in cbmc
@smowton smowton force-pushed the smowton/feature/jbmc-list-symbols branch from 74c9ff9 to 87581eb Compare October 23, 2018 07:15
Copy link
Contributor

@allredj allredj left a comment

Choose a reason for hiding this comment

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

Passed Diffblue compatibility checks (cbmc commit: 87581eb).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/88799241

@smowton smowton merged commit 9686a44 into diffblue:develop Oct 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants