Integrated and Formatted hex-readable flag for SMTChecker#5447
Integrated and Formatted hex-readable flag for SMTChecker#5447ChrisCates wants to merge 4 commits intoargotorg:developfrom ChrisCates:ChrisCates/SMT
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #5447 +/- ##
===========================================
+ Coverage 88.08% 88.09% +<.01%
===========================================
Files 308 309 +1
Lines 31239 31294 +55
Branches 3749 3754 +5
===========================================
+ Hits 27518 27567 +49
Misses 2467 2467
- Partials 1254 1260 +6
|
|
Hmm... Despite code coverage increasing slightly... It seems to say the check has failed...?~ |
| BOOST_CHECK_EQUAL(formatNumberReadable((u256)0x7ffffffff), "0x08 * 2**32 - 1"); | ||
| BOOST_CHECK_EQUAL(formatNumberReadable((u256)0x7fffffffff), "0x80 * 2**32 - 1"); | ||
| BOOST_CHECK_EQUAL(formatNumberReadable((u256)0x7ffffffffff), "0x08 * 2**40 - 1"); | ||
| BOOST_CHECK_EQUAL(formatNumberReadable((u256)0x88000000), "0x88 * 2**24"); |
There was a problem hiding this comment.
For small numbers like all the above I think it would make more sense displaying them in decimal.
There was a problem hiding this comment.
Say for <= 65536 it definitely makes sense.
There was a problem hiding this comment.
Okay great! I will get you these changes later today!~
Thanks for being quick and responsive!~
There was a problem hiding this comment.
Hey, just want to confirm here.
65536 = 0x10000
We were originally going with:
16777216 = 0x1000000
Would you like it at:
4294967296 = 0x100000000
instead? Im pushing up changes right now.
There was a problem hiding this comment.
Ah never mind, decimal would be nice if it would use a power of 10 on the right hand side.
There was a problem hiding this comment.
Could you clarify with an example output? 😺
There was a problem hiding this comment.
If what I provided (with the revisions) are good to go.
Please let me know and I will squash the commits for you and we can merge it into develop.
Feel free to ping @chriseth or @ryan-shea if you find any other issues!~
|
I'm not sure about the command line flag. I think the SMTChecker should use the precise hex readable format by default. |
Is there anything else you would like done @leonardoalt? If not, I can update the flag to be the inverse. True instead of false, then squash commits to finalize the PR. |
|
I actually have two comments here:
|
leonardoalt
left a comment
There was a problem hiding this comment.
@ChrisCates Please modify the PR such that:
- The SMTChecker always uses
formatNumberReadable -
formatNumberReadabletakes a parameter (defaultfalse) that tells whether it should truncate - No command line flag is used
Then I think it should be fine to merge!
|
@leonardoalt, I have a question in regards to the parameter for In what cases does the parameter become true? Without the command line flag... I'm not sure in which contexts it will change. Also, I assume the revised formatting would be something like: template <class T>
inline std::string formatNumberReadable(T _value, bool readable = false)
{
if (readable == false) return formatNumber(_value);
...Once this is clarified, I will push and squash commits. I am also going to start a new PR to get unit tests for |
|
Closing for #5476 instead. |
Description
As part of issue #4648.
I have integrated a
--hex-readableflag for the SMT/Z3 Theorem checker.Feel free to review anytime @chriseth and @ryan-shea.
Checklist