Skip to content

Conversation

@xbauch
Copy link
Contributor

@xbauch xbauch commented Jan 31, 2020

and a reference in the documentation.

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

```

The specification of the XML trace output can be found here:
[XML Specification](https://raw.githubusercontent.com/diffblue/cbmc/develop/doc/assets/xml_spec.tex)

Choose a reason for hiding this comment

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

This should contain information on how the file is intended to be processed. I think this should work with most LaTeX backends with -shell-escape?

Choose a reason for hiding this comment

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

Also, maybe even consider producing it as part of our documentation somehow? The LaTeX itself isn’t super human readable unprocessed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah 🚫 there must be either a script or a readme next to it explaining how to build it.

@majakusber
Copy link

majakusber commented Jan 31, 2020

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #5222   +/-   ##
========================================
  Coverage    67.43%   67.43%           
========================================
  Files         1157     1157           
  Lines        95351    95351           
========================================
  Hits         64303    64303           
  Misses       31048    31048           
Flag Coverage Δ
#cproversmt2 42.67% <ø> (ø) ⬆️
#regression 63.96% <ø> (ø) ⬆️
#unit 31.95% <ø> (ø) ⬆️

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 5b4bcc6...e6c2933. Read the comment docs.

@majakusber
Copy link

Why is my face on the codecov comment? 👀

@xbauch
Copy link
Contributor Author

xbauch commented Feb 5, 2020

Why is my face on the codecov comment?

Wait. You didn't post the coverage report here?

@majakusber
Copy link

Nope, never seen this PR, never did anything with codecov 🤷‍♀️ I only learned about this because I got emails from github after Thomas added his comment (but not earlier).

Copy link
Collaborator

@martin-cs martin-cs left a comment

Choose a reason for hiding this comment

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

I appreciate that documentation is often a thankless task so forgive me if I am making this harder but would it be possible to produce the spec as an XSD file or something so that we add a regression test that validates the output of CBMC against it. That way when someone changes the format we should at least have a flag to update the documentation.

@xbauch xbauch force-pushed the feature/xml-spec branch 2 times, most recently from 6b53770 to 1bd6f05 Compare February 7, 2020 12:41
petr-bauch and others added 2 commits February 24, 2020 17:21
and a reference in the documentation.
Also replace some latex instructions that pandoc can’t handle with
some it can.
Note that this is based on observed outputs from cbmc, there is currently no
mechanism in place that enforces CBMC to conform to this.

We have discussed adding a regression test pass for checking that at least our
own regression tests conform to the spec. In a future PR.
@codecov-io
Copy link

codecov-io commented Feb 25, 2020

Codecov Report

Merging #5222 into develop will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #5222   +/-   ##
========================================
  Coverage    67.42%   67.42%           
========================================
  Files         1162     1162           
  Lines        95620    95620           
========================================
  Hits         64476    64476           
  Misses       31144    31144
Flag Coverage Δ
#cproversmt2 100% <ø> (+57.4%) ⬆️
#regression 63.95% <ø> (ø) ⬆️
#unit 31.89% <ø> (ø) ⬆️
Impacted Files Coverage Δ
src/solvers/smt2/smt2_dec.h 100% <0%> (ø) ⬆️
src/solvers/smt2/letify.cpp 100% <0%> (ø) ⬆️
src/solvers/floatbv/float_bv.h 100% <0%> (ø) ⬆️
src/solvers/smt2/smt2irep.cpp 88% <0%> (ø) ⬆️
src/solvers/floatbv/float_bv.cpp 74.36% <0%> (ø) ⬆️
src/solvers/smt2/smt2_dec.cpp 67.69% <0%> (ø) ⬆️
src/solvers/smt2/letify.h 100% <0%> (ø) ⬆️
src/solvers/smt2/smt2_conv.h 100% <0%> (ø) ⬆️
src/solvers/smt2/smt2_conv.cpp 57.23% <0%> (ø) ⬆️
src/solvers/flattening/pointer_logic.cpp 89.39% <0%> (ø) ⬆️
... and 17 more

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 1f2f1fa...44879df. Read the comment docs.

@hannes-steffenhagen-diffblue
Copy link
Contributor

Turning it off and turning it on again

The specification of the XML trace output can be found here: [XML
Specification](https://raw.githubusercontent.com/diffblue/cbmc/develop/doc/assets/xml_spec.tex)
and can be build by `pdflatex -shell-escape xml_spec.tex`. Alternatively, you
view it in Markdown [here](../../../assets/xml_spec).
Copy link
Contributor

Choose a reason for hiding this comment

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

This link doesn't seem to work, I think you only need to go up two directories

Suggested change
view it in Markdown [here](../../../assets/xml_spec).
view it in Markdown [here](../../assets/xml_spec).

Copy link
Contributor

@hannes-steffenhagen-diffblue hannes-steffenhagen-diffblue Mar 4, 2020

Choose a reason for hiding this comment

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

The link doesn’t work because it links to the develop documentation, which doesn’t have this yet I think.

Copy link
Contributor

@chrisr-diffblue chrisr-diffblue left a comment

Choose a reason for hiding this comment

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

This looks like a great improvement.

@hannes-steffenhagen-diffblue hannes-steffenhagen-diffblue merged commit 8db2b8b into diffblue:develop Mar 4, 2020
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.

8 participants