Skip to content
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

Consistently use unsigned/size_t in big-int [blocks: #2310] #2452

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

tautschnig
Copy link
Collaborator

No description provided.

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: a323dd2).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/77183230
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).

@kroening
Copy link
Member

The index shifts are certainly scary.

Equally scary is the idea that most of these 'unsigned' should likely be size_t, even though that means that we worry about an integer that needs more than 2^32 bits to represent.

@allredj
Copy link
Contributor

allredj commented Jun 26, 2018

Could we have unit tests for these, to make sure that we're not breaking any corner case?

@tautschnig tautschnig assigned tautschnig and unassigned kroening Jul 6, 2018
@tautschnig tautschnig changed the title Consistently use unsigned in big-int Consistently use unsigned/size_t in big-int Sep 4, 2018
@tautschnig
Copy link
Collaborator Author

I have added a commit to move to size_t and have also enabled a unit test for BigInt.

@tautschnig tautschnig assigned kroening and unassigned tautschnig Sep 4, 2018
@tautschnig tautschnig changed the title Consistently use unsigned/size_t in big-int Consistently use unsigned/size_t in big-int [blocks: #2310] Nov 7, 2018
@tautschnig tautschnig assigned tautschnig and unassigned kroening Nov 7, 2018
@tautschnig tautschnig force-pushed the vs-big-int branch 2 times, most recently from 207ea9a to 89f99fa Compare November 11, 2018 22:13
@tautschnig tautschnig assigned kroening and unassigned tautschnig Nov 11, 2018
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: e5501eb).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/91047441

tautschnig added a commit that referenced this pull request Nov 14, 2018
Add author information to big-int implementation file [blocks: #2452]
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: d63c049).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/91344288

tautschnig added a commit that referenced this pull request Dec 19, 2018
Move big-int unit test to unit/ folder and make it a CATCH-style test [blocks: #2452]
@NlightNFotis
Copy link
Contributor

Hi @tautschnig, is this something you plan on merging soon?

If you'd like it merged but don't have the time to work on this, is there something I can do to get this in a better shape to merge?

@tautschnig tautschnig self-assigned this Nov 20, 2022
@TGWDB
Copy link
Contributor

TGWDB commented May 3, 2023

Closing due to age (no further comment on PR content), please reopen with rebase on develop if you intent to continue this work.

@TGWDB TGWDB closed this May 3, 2023
@tautschnig tautschnig reopened this Feb 7, 2025
@tautschnig tautschnig requested a review from a team as a code owner February 7, 2025 13:59
@tautschnig tautschnig removed their assignment Feb 7, 2025
@tautschnig
Copy link
Collaborator Author

@kroening This has been rebased and I hope to have addressed all past concerns.

Copy link

codecov bot commented Feb 7, 2025

Codecov Report

Attention: Patch coverage is 92.91339% with 9 lines in your changes missing coverage. Please review.

Project coverage is 79.59%. Comparing base (66004dc) to head (c7a1826).

Files with missing lines Patch % Lines
src/big-int/bigint.cc 92.62% 9 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #2452      +/-   ##
===========================================
+ Coverage    78.71%   79.59%   +0.88%     
===========================================
  Files         1732     1732              
  Lines       199536   197321    -2215     
  Branches     18281    18214      -67     
===========================================
+ Hits        157057   157059       +2     
+ Misses       42479    40262    -2217     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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