Skip to content

String container stats from the VSD branch#5204

Closed
martin-cs wants to merge 0 commit intodiffblue:developfrom
martin-cs:VSD/string-container-stats
Closed

String container stats from the VSD branch#5204
martin-cs wants to merge 0 commit intodiffblue:developfrom
martin-cs:VSD/string-container-stats

Conversation

@martin-cs
Copy link
Collaborator

@martin-cs martin-cs commented Dec 9, 2019

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

@peterschrammel peterschrammel Dec 9, 2019

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

@martin-cs martin-cs Dec 12, 2019

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
out << ' ';
}
out << kib << si_kibitye_symbol;
out << kib << si_kibibyte_symbol;
Copy link
Member

@peterschrammel peterschrammel Dec 9, 2019

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

@martin-cs martin-cs Dec 12, 2019

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
mathematical_expr.cpp \
mathematical_types.cpp \
memory_info.cpp \
memory_units.cpp \
Copy link
Member

@peterschrammel peterschrammel Dec 9, 2019

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

@martin-cs martin-cs Dec 12, 2019

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
@@ -1,3 +1,11 @@
/*******************************************************************\

Module: Memory units
Copy link
Member

@peterschrammel peterschrammel Dec 9, 2019

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

@martin-cs martin-cs Dec 12, 2019

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

martin-cs commented Dec 12, 2019

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

@hannes-steffenhagen-diffblue hannes-steffenhagen-diffblue left a comment

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

martin-cs commented Feb 6, 2020

@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

@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

+2 more reviewers

@allredj allredj allredj left review comments

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

Reviewers whose approvals may not affect merge requirements

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