Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More detail about candidates in error messages for ambiguous overloaded calls #13812

Closed
cameel opened this issue Dec 16, 2022 · 3 comments
Closed
Labels
annoys users 😢 medium effort Default level of effort medium impact Default level of impact must have eventually Something we consider essential but not enough to prevent us from releasing Solidity 1.0 without it.

Comments

@cameel
Copy link
Member

cameel commented Dec 16, 2022

This came up recently on the forum: Why are implicit string to bytes conversions allowed in external calls but not internally?

Errors that the compiler reports when there is more than one candidate for an overload but none of them is quite right don't have enough detail to make it clear to the user why they don't match. The error is just No unique declaration found after argument-dependent lookup and a list of candidates.

When there is exactly one candidate, the compiler type-checks its arguments and says what exactly is wrong: too many arguments, no implicit conversion, etc. The same information should be printed for each candidate in case where there's more than one.

Note that the same mechanism should separately be applied to user-defined operators as well, since for them overloading is still possible but works differently than for functions (#13790/#13718).

Example - multiple candidates

contract C {
    function f() public {}
    function f(bytes4) public {}
    function f(bytes calldata) public {}

    function test() external {
        f("Hello World");
    }
}

The compiler points out the problem and candidates but does not explain what's wrong with each of them. Especially in this example the second and third overload look like they should match and it may be hard to figure out why they don't.

Error: No matching declaration found after argument-dependent lookup.
 --> test.sol:7:9:
  |
7 |         f("Hello World");
  |         ^
Note: Candidate:
 --> test.sol:2:5:
  |
2 |     function f() public {}
  |     ^^^^^^^^^^^^^^^^^^^^^^
Note: Candidate:
 --> test.sol:3:5:
  |
3 |     function f(bytes4) public {}
  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Note: Candidate:
 --> test.sol:4:5:
  |
4 |     function f(bytes calldata) public {}
  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Example - one candidate

For each candidate in insolation you get a much clearer message:

function f() public {}

Error: Wrong argument count for function call: 1 arguments given but expected 0.
 --> test.sol:5:9:
  |
5 |         f("Hello World");
  |         ^^^^^^^^^^^^^^^^

function f(bytes4) public {}

Error: Invalid type for argument in function call. Invalid implicit conversion from literal_string "Hello World" to bytes4 requested. Literal is larger than the type.
 --> test.sol:5:11:
  |
5 |         f("Hello World");
  |           ^^^^^^^^^^^^^

function f(bytes calldata) public {}

Error: Invalid type for argument in function call. Invalid implicit conversion from literal_string "Hello World" to bytes calldata requested.
 --> test.sol:5:11:
  |
5 |         f("Hello World");
  |           ^^^^^^^^^^^^^
@cameel cameel added annoys users 😢 medium effort Default level of effort medium impact Default level of impact must have eventually Something we consider essential but not enough to prevent us from releasing Solidity 1.0 without it. labels Dec 16, 2022
@cameel
Copy link
Member Author

cameel commented Dec 16, 2022

Related bug #13813 that makes this even worse when the function is called via member access.

@cameel cameel changed the title More detail in error messages for ambiguous overloaded calls More detail about candidates in error messages for ambiguous overloaded calls Dec 16, 2022
@cameel
Copy link
Member Author

cameel commented Dec 16, 2022

And an earlier related issue for modifiers #12332.

@cameel
Copy link
Member Author

cameel commented Dec 17, 2022

Looks like we actually have an issue for this (#9607), so this is a duplicate. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
annoys users 😢 medium effort Default level of effort medium impact Default level of impact must have eventually Something we consider essential but not enough to prevent us from releasing Solidity 1.0 without it.
Projects
None yet
Development

No branches or pull requests

1 participant