Skip to content

String container stats from the VSD branch #5204

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 0 commits into from
Closed

String container stats from the VSD branch #5204

wants to merge 0 commits into from

Conversation

martin-cs
Copy link
Collaborator

Like #5202 and #5203 this is taken from the variable-sensitivity-domain branch and split out as separate commits.

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

@martin-cs martin-cs force-pushed the VSD/string-container-stats branch from f2f8e28 to a34fecd Compare December 9, 2019 22:11
peterschrammel
peterschrammel reviewed Dec 9, 2019
View reviewed changes
src/util/memory_units.h Outdated
#include <cstddef>
#include <string>

struct memory_sizet
Copy link
Member

Choose a reason for hiding this comment

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

class ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sure, why not?

src/util/memory_units.cpp Outdated
@@ -80,7 +80,7 @@ std::string memory_sizet::to_string() const
{
out << ' ';
}
out << kib << si_kibitye_symbol;
out << kib << si_kibibyte_symbol;
Copy link
Member

Choose a reason for hiding this comment

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

memory_unit changes should be in previous commit

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Refactored and sqashed.

src/util/Makefile Outdated
@@ -45,6 +45,7 @@ SRC = allocate_objects.cpp \
mathematical_expr.cpp \
mathematical_types.cpp \
memory_info.cpp \
memory_units.cpp \
Copy link
Member

Choose a reason for hiding this comment

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

squash with initial memory_units commit

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Squashed.

src/util/memory_units.cpp Outdated
@@ -1,3 +1,11 @@
/*******************************************************************\

Module: Memory units
Copy link
Member

Choose a reason for hiding this comment

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

Squash

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Squashed

@martin-cs martin-cs force-pushed the VSD/string-container-stats branch from a34fecd to 3209bd8 Compare December 12, 2019 18:49
@martin-cs
Copy link
Collaborator Author

All review comments resolved.

allredj
allredj reviewed Dec 12, 2019
View reviewed changes
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: 3209bd8).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/140864409

hannes-steffenhagen-diffblue
hannes-steffenhagen-diffblue approved these changes Jan 31, 2020
View reviewed changes
Copy link
Contributor

Choose a reason for hiding this comment

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

I’m OK with this per se, but it doesn’t seem to be used anywhere at the moment?

@martin-cs
Copy link
Collaborator Author

@hannes-steffenhagen-diffblue It is used in the fifth VSD patch which is still to be PR'd. I wanted to get the foundations reviewed and ready first.

@martin-cs martin-cs force-pushed the VSD/string-container-stats branch from 3209bd8 to 9d0c0f3 Compare June 15, 2020 15:10
@codecov
Copy link

codecov bot commented Jun 15, 2020 •
edited
Loading

Codecov Report

Merging #5204 into develop will decrease coverage by 0.04%.
The diff coverage is 0.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #5204      +/-   ##
===========================================
- Coverage    68.25%   68.20%   -0.05%     
===========================================
  Files         1180     1181       +1     
  Lines        97722    97786      +64     
===========================================
  Hits         66698    66698              
- Misses       31024    31088      +64
Flag Coverage Δ
#cproversmt2 42.77% <0.00%> (-0.05%) ⬇️
#regression 65.38% <0.00%> (-0.05%) ⬇️
#unit 32.20% <0.00%> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/util/memory_units.cpp 0.00% <0.00%> (ø)
src/util/string_container.cpp 47.50% <0.00%> (-52.50%) ⬇️
src/util/string_container.h 88.88% <0.00%> (-11.12%) ⬇️

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 a6feed2...b8bef97. Read the comment docs.

@martin-cs martin-cs force-pushed the VSD/string-container-stats branch 3 times, most recently from dfbb67a to 2f77994 Compare June 23, 2020 14:58
@martin-cs martin-cs force-pushed the VSD/string-container-stats branch 2 times, most recently from aa59faf to f6c8f14 Compare July 3, 2020 20:08
@martin-cs martin-cs force-pushed the VSD/string-container-stats branch from f6c8f14 to 5f2bca7 Compare August 3, 2020 16:45
@martin-cs martin-cs force-pushed the VSD/string-container-stats branch from 5f2bca7 to 6ee69aa Compare August 17, 2020 16:29
@martin-cs martin-cs force-pushed the VSD/string-container-stats branch from 6ee69aa to b8bef97 Compare August 24, 2020 20:27
@martin-cs martin-cs mentioned this pull request Aug 24, 2020
Merged
@martin-cs martin-cs closed this Sep 1, 2023
@martin-cs martin-cs force-pushed the VSD/string-container-stats branch from b8bef97 to 228b9dd Compare September 1, 2023 14:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@peterschrammel peterschrammel peterschrammel left review comments

@allredj allredj allredj left review comments

@hannes-steffenhagen-diffblue hannes-steffenhagen-diffblue hannes-steffenhagen-diffblue approved these changes

@chrisr-diffblue chrisr-diffblue Awaiting requested review from chrisr-diffblue

@danpoe danpoe Awaiting requested review from danpoe

@kroening kroening Awaiting requested review from kroening

@pkesseli pkesseli Awaiting requested review from pkesseli

@smowton smowton Awaiting requested review from smowton

@tautschnig tautschnig Awaiting requested review from tautschnig

@thk123 thk123 Awaiting requested review from thk123

Assignees

@martin-cs martin-cs

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@martin-cs @peterschrammel @allredj @hannes-steffenhagen-diffblue