Solidity: Constant evaluation computes array length in arbitrary precision

[leonardoalt]

contract C {
  uint constant a = 12;
  uint constant b = 10;
  function f() public pure returns (uint) {
    uint[(a/b)*b] memory x;
    return x.length;
  }
}

This should return 10, returns 12.

[hrkrshnn]

Shouldn't this be a breaking change?

[leonardoalt]

Well, it's already there, and it's buggy.

[leonardoalt]

@hrkrshnn I mean that using constants to declare a static array instead of using a literal is already there. However, it uses the ConstantEvaluator to compute the length, and that's done in reals. Since a and b have types, it should be done on those types, and use integer division instead.

[hrkrshnn]

I guess that people who used this feature would assume that this was the right behavior. So breaking would be more appropriate in this case.

[chriseth]

We can do it in breaking, just to be sure.

[axic]

I'd argue this is a bug, but since we're close to breaking there's no issue not doing it there. If there is a case where it could end up with a shorter array length than expected, then fixing it in 0.7.x would make sense -- do we have such a potential case?

[leonardoalt]

Does anyone think this should be correct behavior?

[axic]

In the documentation we state that only literals are evaluated at arbitrary precision. We don't mention constants there.

From https://solidity.readthedocs.io/en/v0.7.4/types.html?highlight=precision#rational-and-integer-literals:

Number literal expressions retain arbitrary precision until they are converted to a non-literal type (i.e. by using them together with a non-literal expression or by explicit conversion). This means that computations do not overflow and divisions do not truncate in number literal expressions.

But this second paragraph could be made a bit more clearer:

For example, (2800 + 1) - 2800 results in the constant 1 (of type uint8) although intermediate results would not even fit the machine word size. Furthermore, .5 * 8 results in the integer 4 (although non-integers were used in between).

In what case does it end up as uint8? :)

[leonardoalt]

Operations on constants only behave like that inside the length of a static array declaration

[axic]

Btw what do we do now in other places where one term is typed and there are one or more untyped terms (literals). What type do we deduce? The type of the typed term?

[bshastry]

Btw what do we do now in other places where one term is typed and there are one or more untyped terms (literals). What type do we deduce? The type of the typed term?

I think the latter i.e., type of typed term.

I also noticed that the following test throws an error although z is actually a constant that should evaluate to 20 or 0x14. If the array allocation is commented out, the expected value (0x14) is returned.

contract C {
        uint constant x = 2;
        uint constant z = uint(x)*10;
        uint[z] a;

        function f() public pure returns (uint) {
                return z;
        }
}
// ----
// TypeError 5462: (72-73): Invalid array length, expected integer literal or constant expression.

Edit: This is happening presumably because of the same underlying issue. Users of constant evaluator (in this case https://github.com/ethereum/solidity/blob/be02db4950d0d609b2fc57cf1aeb08e983c4185a/libsolidity/analysis/DeclarationTypeChecker.cpp#L253) assuming evaluated constant is a rational number.

Edit 2:

The following test passes

contract C {
        uint constant x = 2;
        uint constant z = x*10;
        uint[z] a;

        function f() public pure returns (uint) {
                return z;
        }
}

because constant evaluator maintains the type of z to be a rational number type.

[leonardoalt]

Oh, shouldn't it error on the declaration of z though? It uses a typed expression, whereas afaik constants can be assigned only from literals or expressions on literals?

[bshastry]

Oh, shouldn't it error on the declaration of z though? It uses a typed expression, whereas afaik constants can be assigned only from literals or expressions on literals?

Typed expressions may be used to assign to constants apparently. The following test passes

contract C {
        uint constant x = 2;
        uint constant z = uint(x)*10;

        function f() public pure returns (uint) {
                return z;
        }
}
// ====
// compileViaYul: also
// ----
// f() -> 0x14

[leonardoalt]

Right, it only needs to be inferable in compile-time.

[bshastry]

Okay, this is weird. The following test passes i.e., constant expressions inside statically sized array brackets [expr] evaluate differently when compared to expression statements.

contract C {
    uint constant x = 2;
    uint constant y = 4;
    uint[x/y*10] a;

    function f() public view returns (uint, uint) {
        return (x/y*10, a.length);
    }
}
// ====
// compileViaYul: also
// ----
// f() -> 0, 5

I would have expected the expression x/y*10 inside the function to be evaluated the same way as the same expression inside array declaration.

Edit: Reading this thread more closely, I think this is what @leonardoalt was referring to

Operations on constants only behave like that inside the length of a static array declaration

[leonardoalt]

@bshastry Yes, in DeclarationTypeChecker::endVisit(ArrayTypeName const& _typeName) it uses ConstantEvaluator(m_errorReporter).evaluate(*length); to compute the length when constant expressions are used inside the [].

[bshastry]

The following is clearly a bug. The constant z evaluates differently depending on where it is used i.e., array length vs vanilla evaluation.

contract C {
        uint constant x = 2;
        uint constant z = x/4*10;
        uint[z] b;

        function f() public view returns (uint, uint) {
                return (b.length, z);
        }
}
// ====
// compileViaYul: also
// ----
// f() -> 5, 0

Edit: However, if the constant involves literals only, the evaluation seems consistent

contract C {
        uint constant z = 2/4*10;
        uint[z] b;

        function f() public view returns (uint, uint) {
                return (b.length, z);
        }
}
// ====
// compileViaYul: also
// ----
// f() -> 5, 5

So I suppose the pre-requisites for the bug are

• constant is assigned from an expression containing another constant
• assignment contains intermediate fractional type
• constant is used in static array

[leonardoalt]

The following is clearly a bug. The constant z evaluates differently depending on where it is used i.e., array length vs vanilla evaluation.

Yes, that's what I meant when I opened the bug haha
But your test shows it a lot better

[bshastry]

TBH I'm a bit concerned if there are contracts that use z and b.length interchangeably

contract C {
        uint constant x = 2;
        uint constant z = x/4*10;
        uint[z] b;

        function f() public view returns (uint, uint) {
                return (b.length, z);
        }
}

[leonardoalt]

TBH I'm a bit concerned if there are contracts that use z and b.length interchangeably

There might be, but in that case I would expect the expression to not have division (which is what breaks the equivalence)

[christianparpart]

The documentation states:

• "For a constant variable, the expression assigned to it is copied to all the places where it is accessed and also re-evaluated each time." (https://solidity.readthedocs.io/en/v0.7.4/contracts.html#constant-and-immutable-state-variables)

This seems not entirely true (I think), as operators are evaluated in via ConstantEvaluator through (e.g.) RationalNumberType::binaryOperatorResult(...) and only the evaluated result as rational number is stored with the resulting type.

So for me it reads like:

uint constant a = 12; // internally stored as rational (12 / 1)
uint constant b = 10; // internally stored as rational (10 / 1)
uint constant c = a / b; // internally stored as rational (12 / 10), evaluated in `RationalNumberType::binaryOperatorResult(...)` for `Token::Div`
uint constant d = c * b; // internally stored as rational (12 / 1), evaluated in `RationalNumberType::binaryOperatorResult(...)` for `Token::Mul`

uint C = a / b; // is  value 1 (due to integer division truncation)
uint D = (a / b) * b; // is value 10

If I understand you correctly, @leonardoalt , it is expected to respect the type of a and b (both uint) when doing the division a / b in the array declaration subscript.

Intuitively I'd expect the same, but what it seems to violate against is that the documentation states "For a constant variable, the expression assigned to it is copied to all the places where it is accessed and also re-evaluated each time". So the current behavior (as per documentation) seems correct to me (correct me if I'm wrong).

@bshady has nice examples where constant expression evaluation differs from runtime expression evaluation. The problem I'm having now what behavior exactly do we want to have?

[chriseth]

@christianparpart The code snippet you gave does not use the constant evaluator. My impression is also that the behaviour you describe is the correct behaviour. The constant evaluator is used for computing lengths in array types:
uint[d] is currently the same as uint[12] , but it should be uint[10], the same as uint[D].