evmc icon indicating copy to clipboard operation
evmc copied to clipboard

cpp: Add support for _uint256be literals

Open axic opened this issue 4 years ago • 6 comments

Only extending the literal tests, because the other tests are covered via bytes32 tests given uint256be is an alias.

axic avatar May 04 '21 22:05 axic

Codecov Report

Merging #596 (dcdfb9f) into master (58913e0) will increase coverage by 0.96%. The diff coverage is 100.00%.

:exclamation: Current head dcdfb9f differs from pull request most recent head 57b3cc3. Consider uploading reports for the commit 57b3cc3 to get more accurate results

@@            Coverage Diff             @@
##           master     #596      +/-   ##
==========================================
+ Coverage   92.84%   93.81%   +0.96%     
==========================================
  Files          23       25       +2     
  Lines        3552     3769     +217     
  Branches      376        0     -376     
==========================================
+ Hits         3298     3536     +238     
- Misses        144      233      +89     
+ Partials      110        0     -110     

codecov[bot] avatar May 04 '21 22:05 codecov[bot]

What will be confusing here is that these literals are required to have exactly 64 hex digits.

chfast avatar May 05 '21 11:05 chfast

Are you going to use it?

I was adding --value support to the evmc tool, that's how I found this, but in the end it is actually not that important.

What will be confusing here is that these literals are required to have exactly 64 hex digits.

Yeah I was wondering if it would make sense make this work on decimal instead. Probably better holding the merge back a bit.

axic avatar May 05 '21 18:05 axic

Yeah I was wondering if it would make sense make this work on decimal instead. Probably better holding the merge back a bit.

Unless there exist some simplified algorithm for decimal literals I'm not aware of, this requires implementing multiplication by 10. https://github.com/chfast/intx/blob/master/include/intx/int128.hpp#L893

chfast avatar May 06 '21 09:05 chfast

@chfast leaning towards that even with base 16 this may be useful

axic avatar May 22 '22 22:05 axic

Rebased. I think this is still useful for unit tests, based on ethereum/tests. I think I needed/wanted it for soltests at the time.

axic avatar Jun 01 '22 16:06 axic