Skip to content

Improve replace_symbolt precondition message #5219

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

karkhaz
Copy link
Collaborator

@karkhaz karkhaz commented Jan 29, 2020

Invariant violations now print out the actual types that were mismatched.

Copy link
Contributor

Choose a reason for hiding this comment

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

Other than formatting I’m OK with this.

old_expr.type() == new_expr.type(),
"types to be replaced should match.\nold type: '" +
old_expr.type().pretty() + "'\nnew.type: '" + new_expr.type().pretty() +
"')");

Choose a reason for hiding this comment

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

Suggested change
"')");
PRECONDITION_WITH_DIAGNOSTICS(
old_expr.type() == new_expr.type(),
"types to be replaced should match.\nold type: ", old_expr.type(),
"new.type: ", new_expr.type());

I’d recommend against the quotes and what not (the output isn’t really machine parseable anyway, and irep outputs are almost always going to be multiline so the quotes don’t really help with human readability either), and anything that has a specialisation for diagnostics_helpert (ireps do), can just be added as an argument and the macro takes care of the concatenation.

@karkhaz karkhaz force-pushed the kk-useful-replace-symbol-assertion branch 2 times, most recently from 9f9a16c to 8649e1e Compare February 10, 2020 20:27
Invariant violations now print out the actual types that were
mismatched.
@karkhaz karkhaz force-pushed the kk-useful-replace-symbol-assertion branch from 8649e1e to 52001fa Compare February 10, 2020 20:46
@majakusber
Copy link

Codecov Report

Merging #5219 into develop will not change coverage by %.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #5219   +/-   ##
========================================
  Coverage    67.43%   67.43%           
========================================
  Files         1157     1157           
  Lines        95352    95352           
========================================
  Hits         64302    64302           
  Misses       31050    31050           
Flag Coverage Δ
#cproversmt2 42.67% <100.00%> (ø) ⬆️
#regression 63.96% <100.00%> (ø) ⬆️
#unit 31.95% <100.00%> (ø) ⬆️

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 778fb97...52001fa. Read the comment docs.

@karkhaz karkhaz merged commit 14676e2 into diffblue:develop Feb 10, 2020
@karkhaz karkhaz deleted the kk-useful-replace-symbol-assertion branch February 10, 2020 22:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants