Ternary operator does not correctly deduce common type for `-1` and `0` to be `int8`

[lukehutch]

Description

Using trinary comparison operators causes type inference to fail.

Environment

• Compiler version: 0.8.14
• Target EVM version (as per compiler settings): unknown
• Framework/IDE (e.g. Truffle or Remix): Hardhat
• EVM execution environment / backend / blockchain client:
• Operating system: Linux

Steps to Reproduce

    function ab(int a, int b) public returns (int) {
        return a < b ? -1 : 0;
    }

Yields

TypeError: True expression's type int8 does not match false expression's type uint8.
    |
16 |         return a < b ? -1
    |                ^ (Relevant source part starts here and spans across multiple lines).

So there are really two problems:

1. The trinary operator's true and false operands are not appropriately unified to int8 (which works for both operands)
2. The trinary operator's true and false operands are not correctly widened from int8 to the return type int.

The solution is a bit ugly:

    function ab(int a, int b) public returns (int) {
        return a < b ? int(-1) : int(0);
    }

[cameel]

2. The trinary operator's true and false operands are not correctly widened from int8 to the return type int.

This part works correctly. Try this:

function ab(int a, int b) public returns (int) {
    return a < b ? -1 : int8(0);
}

1. The trinary operator's true and false operands are not appropriately unified to int8 (which works for both operands)

Yes, I think this does not work as it should. The compiler should be deducing that the common type is int8 but for some reason it decides that they're incompatible. On the other hand for array literals (which have the same deduction mechanism) it works correctly - this compiles without errors:

int8[2] memory x = [-1, 0];

So I agree, I think it's a bug in the sense that these mechanisms should work the same way and they clearly don't.

[cameel]

@wechman You might be interested in this since you're working on array literals right now.

[wechman]

@cameel a type deduction mechanism for inline arrays the compiler currently use surly is not perfect and a bit tricky. To see that, please just try to change order of array elements in your example so you have: int8 [2] memory x = [0, -1]; Such a small change and compilation end up with an error. That's because the compiler relays on a type of the first element of an array literal. Luckily, this is going to be changed soon. Nevertheless, I am not sure if deduction mechanisms of the array literals and ternary operator have that much in common. But, I agree that the only problem we have in this ticket is the compiler inability to deduce int8 as a ternary operator operation type.

[cameel]

Interesting! In any case, even if the implementation is actually separate I think that both cases should follow the same rules from user's standpoint. Otherwise it's confusing to users. So when you're done with arrays I think we should also look at the ternary operator and bring these two mechanisms in sync.

[wechman]

I have looked over the documentation and found this:

• The type in which operator is performed is a common type of the operands and can be found only if one operand type can be implicitly converted to the other.
• For literals, the compiler uses their mobile type to find common type.
• Mobile type is the smallest type that can hold the value. Unsigned types of the same bit-width are considered “smaller” than the signed types.

In view of the above, -1 is being converted to int8. At the same time 0 is being converted to uint8 cause unsigned type is considered to be smaller then signed. Since that moment, the compiler uses mobile types. Because the compiler cannot guarantee lossless conversion between int8 and uint8, the code compilation fails. This is exactly why explicit conversion is necessary in this case.

@cameel For now I can see two options to address this problem. First one is to deduce common type based on literals instead of their mobile types. This can help to fix this ticket and similar cases, but also can be tricky since does not concern types other than literals (e.g. condition ? variable_a : variable_b). However, do we want to change anything but literals? The second option would be to provide a new mechanism to deduce common type of numerical types. It should come with a type promotion algorithm. For instance int8 and uint8 could be promoted to int16. Does it make sense?

[cameel]

In view of the above, -1 is being converted to int8. At the same time 0 is being converted to uint8 cause unsigned type is considered to be smaller then signed. Since that moment, the compiler uses mobile types.

Hmm... But this means that we're not really applying that rule for other operators, right? Otherwise something as simple as -1 + 0 would be a compilation error. So the docs seem wrong about literals.

For now I can see two options to address this problem. First one is to deduce common type based on literals instead of their mobile types.

Yeah, if I understand things correctly, we're already doing that for other operators anyway. The ternary seems to be the only odd one.

This can help to fix this ticket and similar cases, but also can be tricky since does not concern types other than literals (e.g. condition ? variable_a : variable_b). However, do we want to change anything but literals?

I think it's not an issue. For example this currently fails as well:

    int8 x = -1;
    uint8 y = 0;
    x + y;

Error: Operator + not compatible with types int8 and uint8
 --> test.sol:4:5:
  |
4 |     x + y;
  |     ^^^^^

even though the same operation on literals is valid.

The second option would be to provide a new mechanism to deduce common type of numerical types. It should come with a type promotion algorithm. For instance int8 and uint8 could be promoted to int16. Does it make sense?

I would not be against that but I have a suspicion that such a promotion might have been omitted on purpose. Most of the time you want to stay within an unsigned type when possible. I can't give any example off the top of my head where the automatic conversion to signed would be bad, but I have a feeling that it might be possible to find one.

[wechman]

@cameel I created a draft with the simplest solution for the issue. Please let me know if like it.