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

Ranked overload resolution #1256

Open
chriseth opened this issue Oct 20, 2016 · 13 comments
Open

Ranked overload resolution #1256

chriseth opened this issue Oct 20, 2016 · 13 comments
Labels
language design :rage4: Any changes to the language, e.g. new features 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. needs design The proposal is too vague to be implemented right away

Comments

@chriseth
Copy link
Contributor

Currently, functions are selected as overload candidates if all arguments can be implicitly converted to the expected types. If there is not exactly one candidate, resolution fails.
This makes it impossible to call different functions depending on whether one argument is a storage or a memory array, because storage arrays can be implicitly converted to both storage and memory arrays.
We should add a ranking of candidates depending on which conversions have to take place.
For this, a conversion from e.g. storage to memory should be "worse" than a widening of an integer type which in turn is worse than no conversion at all.

@axic
Copy link
Member

axic commented Oct 25, 2016

This is needed so much!

The case when there's an exact match with other convertible types should be fixed very soon. I've this problem uints:

contract A {
  function a(uint32 x) {}
  function a(uint x) {}
  function b() {
    uint32 c = 1;
    a(c);
  }
}

@apmilen
Copy link
Contributor

apmilen commented Apr 21, 2017

Hi, I just encountered this issue and was wondering what the plan is for incorporating a fix?

@chriseth
Copy link
Contributor Author

We have to define the order of resolution for every possible conversion.

@dmihal
Copy link

dmihal commented Jun 10, 2018

Just bumped into this issue, with function foo(uint152 x) and function foo(address x).

Is this issue still unresolved?

@chriseth
Copy link
Contributor Author

I'll add it to 0.6.0.

@Jaime-Iglesias
Copy link
Contributor

What is the current thinking around this issue ?

@chriseth
Copy link
Contributor Author

chriseth commented Aug 3, 2021

I think this could be done in a non-breaking way (because after this change, I think more code compiles than before) - can someone please check?

@chriseth
Copy link
Contributor Author

chriseth commented Aug 3, 2021

My proposal for a simple implementation is: If there is a unique function where the argument types are equal (including storage location), use this one. Otherwise do overload resolution as we currently do it.

@cameel
Copy link
Member

cameel commented Feb 7, 2022

I have an idea. What if, instead of adding implicit rules on overload resolution we added a way to clarify the type of an argument?

My proposal would be to introduce a special conversion operator, let's call it exact(), that would make its argument not implicitly convertible to anything. Example:

function foo(uint256 x) {}
function foo(uint32 x) {}
function foo(address x) {}
function foo(string memory x) {}
function foo(string storage x) {}

function test() {
    uint32 x;
    foo(exact(x));          // calls foo(uint32)
    foo(exact(uint(x)));    // calls foo(uint256)
    foo(x);                 // ERROR: ambiguous

    address a;
    foo(a);                 // calls foo(address) directly
    foo(payable(a));        // calls foo(address) through implicit conversion
    foo(exact(payable(a))); // ERROR: foo(payable) does not exist; implicit conversion to address not allowed

    string storage s;
    foo(s);                 // ERROR: ambiguous
    foo(exact(s));          // calls foo(string storage)

    // If we introduce the copyof operator:
    foo(copyof s);          // ERROR: ambiguous
    foo(exact(copyof s));   // calls foo(string memory)
    foo(exact(memory(s));   // calls foo(string memory) - possible alternative if we don't
}

The operator would affect the expression rather than "stick" to the value. When you copy/assign a value, it does not preserve the "exactness". I.e. after uint32 y = exact(uint32(x)), y is again implicitly convertible to uint.

Some alternative syntax ideas:

foo(x, exact payable(a), y);
foo(x, exact<payable>(a), y);
foo(x, exact|payable(a)|, y);
foo(x, |payable(a)|, y);

foo(x, only(payable(a)), y);
foo(x, explicit(payable(a)), y);
foo(x, noconv(payable(a)), y);
foo(x, asIs(payable(a)), y);

// We could also allow making variables permanently exact:
address payable exact constant b;
foo(x, b, y);

@ekpyron
Copy link
Member

ekpyron commented Feb 7, 2022

I would actually say we can relax this even more radically.
Implicit conversions induce an ordering of types, e.g. uint8 < uint16 < uint64 < uint256.
We can actually use this order for this.
So in

function f(uint64) {}
function f(uint256) {}
f(uint8(0));

we can just disregard f(uint256), since uint64 is implicitly convertible to uint256 and thereby uint64 is strictly more specialized than uint256.

@ekpyron
Copy link
Member

ekpyron commented Feb 7, 2022

So in general, if I have two overload choices f(A) and f(B), I can disregard f(B) exactly if A is implicitly convertible to B, but B is not implicitly convertible to A. That works even if the argument type that triggered the overload resolution is merely implicitly convertible to A, but not identical to A.

@cameel cameel added 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. needs design The proposal is too vague to be implemented right away labels Sep 14, 2022
@PaulRBerg
Copy link
Contributor

Chiming in to cast my support for this.

As I explained here, I find it a bit confusing that the compiler cannot disambiguate between two functions f(uint64) and f(uint256).

Overloading functions with different integer width is super common when writing tests with Foundry.

@cameel
Copy link
Member

cameel commented Jan 19, 2023

Just wanted to note that when solving this we should remember that integers are not the only case here. For example #13879 brought up a related case with function pointers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
language design :rage4: Any changes to the language, e.g. new features 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. needs design The proposal is too vague to be implemented right away
Projects
None yet
Development

No branches or pull requests

9 participants