Skip to content

Fix/redirect exception handler output 3 #3952

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

martin-cs
Copy link
Collaborator

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

This is #3921 / #3951 redone to use message_handlert as suggested by @tautschnig plus the corresponding updates to the Java tools as #3897 . It fixes and enables the tests in #3902 . This should reduce the risk of output, especially exceptions, winding up on std::cerr when using JSON or XML output.

martin added 5 commits January 25, 2019 19:59
Getting parse_option_baset to use the same message_handlert as
the rest of the application should mean that error message,
especially those from default exception handlers will be formatted
correctly.  So, if you use --json-ui then the exception messages
will be in JSON rather than just dumped on std::cerr.

There is an issue with object constructors; as the
message_handlert is a local variable in most programs, it is not
initialised when it is passed as a reference.  A comment explaining
this problem is given and the issue is no worse than it already was.
This is just the reformattings required by the previous
commit.
These should check that exception messages are output
wrapped in JSON rather than just dumped on std::cerr.
This is a copy of an earlier patch to the CBMC tools which replaced
patchy and inconsistent exception handling in each tool with the
comprehensive set of catch handlers in parse_options_baset.
@martin-cs martin-cs force-pushed the fix/redirect-exception-handler-output-3 branch from a1c0def to a959df9 Compare January 25, 2019 20:01
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.

Someone doesn't like force-pushing ;-)

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.

🚫
This PR failed Diffblue compatibility checks (cbmc commit: a959df9).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/98664566
Status will be re-evaluated on next push.
Please contact @peterschrammel, @thk123, or @allredj for support.

Common spurious failures:

  • 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).

The incompatibility may have been introduced by an earlier PR. In that case merging this
PR should be avoided unless it fixes the current incompatibility.

@martin-cs
Copy link
Collaborator Author

Paging @peterschrammel @thk123 and @allredj as requested by the TG failure. My guess is that you need one extra argument to the constructor of parse_option_baset and in return you get better integration of exception messages.

@martin-cs
Copy link
Collaborator Author

@peterschrammel has spoken for TestGen so ...

@martin-cs martin-cs merged commit d52cd0d into diffblue:develop Jan 28, 2019
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.

7 participants