solidity::util::formatNumberReadable signed integer support

[leonardoalt]

Currently that function does not support signed values, causing, for example:
Warning 3944: (110-115): Underflow (resulting value less than -57896044618658097711785492504343953926634992332820282019728792003956564819968) happens here.
That number is actually -2**255, so it could be better formatted.

[axic]

Probably the same issue: #9479 (comment)

[Neyromancer]

Hi, i will try to work on it.

[Neyromancer]

Hi, just to clarify. Should the number -57896044618658097711785492504343953926634992332820282019728792003956564819968
be transformed to -2**255
or it can also be represented as
-0x80 * 2**248?
Asking because the number 57896044618658097711785492504343953926634992332820282019728792003956564819968 is now transformed into 0x80 * 2**248.

[cameel]

In my opinion, if that's the current behavior in the positive case then doing a similar thing in the negative one is fine as well. It's still some improvement.

But, it would be best to just change it in both cases to -2**255/2**255. Unless there's some technical reason preventing this - I haven't really looked at the current implementation yet so there might be something I'm not aware of.

Also note that after implementing it you should update expectations in tests because that's where these numbers are used currently. But don't do it manually - you can just run isoltest and accept the changes it proposes (see Writing and Running Syntax Tests in the docs in case you haven't yet).

[Neyromancer]

Actually, i think it is technically possible to move from 0x80 * 2 ** 248 to 2 ** 255 so i will try to implement it.

I just was not sure whether to follow the current implementation or to do it in the way it was written in the task, i.e. which number representation to take as a basiс representation. But i understand now.
By the way thank you for the link:)

[Neyromancer]

if this problem does not block anything. i am still working on it. it took a bit more than expected but guess it is only due to personal reasons.

[cameel]

There's no hurry. It would be pretty helpful, especially in cases like #10776 (comment) but fixing this is not a priority right now so you can work on it at your own pace.

[cameel]

@Neyromancer Are you still working on it? Do you need any help?

[Ruko97]

If this issue hasn't been resolved yet, I am interested to work on it.

[cameel]

It's up for the taking. But I'd recommend the other issue you commented on (#9926) because it's more visible to users and should be more satisfying to fix.

[Ruko97]

Are signed integers implemented in the compiler using u256 as well, or is there another datatype for representing signed integers?

[cameel]

We have s256 as well.

[cameel]

But to be clear, this function is not limited to u256/s256. See for example this place in BMC::checkUnderflow(), where it's called. It takes the type from the expression so it can be any of the integer types provided by TypeProvider.

[Ruko97]

It seems that signed integer was not supported in formatNumberReadable because bigint doesn't support the right shift operator. Should i replace the right shift operators with division for all types of integers, or just use division for bigint in a separate condition and keep the right shift operator for the rest of the integer types?

[cameel]

@nikola-matic @matheusaaguiar What's the status here? Did these merged PRs fix this issue or is there somehing left to do?

[nikola-matic]

@nikola-matic @matheusaaguiar What's the status here? Did these merged PRs fix this issue or is there somehing left to do?

Should be, but @matheusaaguiar spent a lot more time on this, so let's wait for him to chime in.

[matheusaaguiar]

The mentioned PRs have fixed this fixed this issue.