hyperledger / solang

Solidity Compiler for Solana and Polkadot
https://solang.readthedocs.io/
Apache License 2.0
1.23k stars 206 forks source link

Make multidimensional arrays work #932

Open LucasSte opened 2 years ago

LucasSte commented 2 years ago

Our support for multidimensional arrays is broken and the semantics Solidity uses for them is confusing.

Here are some facts to discuss:

  1. I understand int[4][2] vec as a matrix of four rows and two columns, but in Solidity it is a two-element array of int[4].
  2. Should vec[1] index the last or the first dimension? Should it return int[2] or int[4]?
  3. In Solidity, empty square brackets mean that everything that preceded it is the type of each element of a dynamic array. Thus, int[3][4][] vec is a dynamic vector of int[3][4] elements. How do we treat dynamic dimensions that are not the outer ones?
LucasSte commented 2 years ago

@xermicus and @seanyoung What do you think?

xermicus commented 2 years ago

I understand int[4][2] vec as a matrix of four rows and two columns

What? I am 101% with you!

Should vec[1] index the last or the first dimension? Should it return int[2] or int[4]?

So it looks like the order how you access nested arrays is inverted? In that case vec[1] should return int[2].. What does solc produce in this case?

How do we treat dynamic dimensions that are not the outer ones?

This is bit mind stretching.. it means int[2][3][][4] would be 4 elements each containing a dynamic number of 3 rows / 2 columns matrices?

LucasSte commented 2 years ago

This should work, but it is not working now:

    function middle_dyn() public pure returns (uint32) {
        uint8[3][][2] memory vec;
        vec[0] = new uint8[3][](1);
        vec[0][0] = [uint8(1), 2, 3];
        vec[1] = new uint8[3][](2);
        vec[1][0] = [uint8(4), 5, 6];
        vec[1][1] = [uint8(7), 8, 9];

        return vec[0].length;
    }
seanyoung commented 1 year ago

I don't think we can change the syntax.

Possibly we can introduce new syntax with sane ordering. I don't know what that would look like - any suggestions?

LucasSte commented 1 year ago

This case is also broken, but should work after we deploy this feature:


contract testing {

    function test(uint32 dim) public pure returns (uint32[3][][4] memory) {
        uint32[3][][4] memory vec;
        vec[0] = new uint32[3][](dim);
        return vec;
    }

}
LucasSte commented 1 year ago

I just found another inconsistency. Even though indexing in Solidity is reversed for dynamic arrays (see this source), indexing for fixed length arrays happens in the same order as the declared dimensions, i.e. the first bracket indexes the first dimension, so the snippet below prints 2 and 4. This is consistent with what I have tested for Solc.

function multiDimArrays() public pure {
        int8[2][3] memory vec = [[int8(1), 2], [int8(4), 5], [int8(6), 7]];
        print("{}".format(vec[0][1]));
        print("{}".format(vec[1][0]));
}

This adds up in the mess around arrays in Solidity.

LucasSte commented 1 year ago

This adds up in the mess around arrays in Solidity.

I first thought the issue I mentioned in the previous comment was a problem in Solang, but solc indexes the same way as we do.

seanyoung commented 1 year ago

Multidimensional arrays in Solidity is confusing every time I look at them. We should add some notes in our documentation before we resolve this issue.

LucasSte commented 1 year ago

This contract does not work:

contract caller {
    event Event1(int24, uint32);

    function myFunc(int24 a, uint32[2][] b, uint32[2][4] d, uint32[][2] e) public {
        emit Event1(a, b[0][1] + d[1][2] - e[1][0]);
    }
}