hyperledger / solang

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

Parse and resolve fails on overloaded functions depending on order of declaration #1534

Closed bkushigian closed 8 months ago

bkushigian commented 11 months ago

Describe the bug Overloaded function resolution can trigger a failure depending on order of function declarations. I have a minimal reproducible version of this bug here. To reproduce, run cargo run -- testcase/overloaded_order_matters/*.sol.

For instance, the following code works fine for solc and solang's parse_and_resolve:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;

contract Overloaded {
    // Version 1
    function f(uint n) internal pure returns (uint) {
        return n;
    }

    // Version 2
    function f(uint n, uint x) internal pure returns (uint) {
        return n + x;
    }

    // Version 3
    function f(int n, uint x) internal pure returns (int) {
        uint result = f(uint256(n), x);
        return n > 0 ? int(result) : -int(result);
    }
}

However, parse_and_resolve fails to handle this case, where the final two functions are reversed:

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.0;

contract Overloaded {
    // Version 1
    function f(uint n) internal pure returns (uint) {
        return n;
    }

    // Version 3
    function f(int n, uint x) internal pure returns (int) {
        uint result = f(uint256(n), x);
        return n > 0 ? int(result) : -int(result);
    }

    // Version 2
    function f(uint n, uint x) internal pure returns (uint) {
        return n + x;
    }
}

Running solang here causes the following error:

=====  Error: testcase/overload_order_matters/Overloaded3.sol  =====
error: function expects 1 arguments, 2 provided
   ┌─ /Users/benku/solang_parse/testcase/overload_order_matters/Overloaded3.sol:12:23
   │
12 │         uint result = f(uint256(n), x);
   │                       ^^^^^^^^^^^^^^^^

I tried all 6 possible orders of function declarations and this is the only one that fails 🤔. I also wasn't able to make this trigger with only two functions, so I think this is a minimal test case?

seanyoung commented 11 months ago

This is a bug in our overloaded function resolution. It really needs to be re-written.

707 is related

seanyoung commented 11 months ago

Probably need to wait for #1517 to be merged before we can tackle this