Vyper: VIP: Function overloading for interface compatibility

[maurelian]

Simple Summary

Function name overloading appears to be a necessity for compatibility in this ecosystem

Abstract

• Solidity (being the dominant language) is used to define standard contract interfaces
• Solidity allows function overloading, so that two functions can have the same name, but take different arguments
• Per the abi spec, function identifiers are determined by the name and arguments

Therefore, any solidity interface containing multiple functions with the same name is impossible to implement in Vyper

Motivation

To achieve adoption, popular contract standards must be possible to implement in Vyper. For example, ERC721 contains both:

1. function safeTransferFrom(address _from, address _to, uint256 _tokenId, bytes data) external payable;
2. function safeTransferFrom(address _from, address _to, uint256 _tokenId) external payable;

Compiling a vyper file containing that interface will throw: vyper.exceptions.VariableDeclarationException: Duplicate function or event name: safeTransferFrom

Specification

• Change the Duplicate function or event name error to a warning
• Other?

Backwards Compatibility

I don't think there are any such issues, given that this expands the allowable source code space.

Copyright

Copyright and related rights waived via CC0

[fubuloubu]

This should be possible to allow. The check for duplicate definition should maybe check the function signature hash instead of the function name.

[jacqueswww]

I would caution this, overloading functions is one of our core principals we have stated we want to avoid.
And I do believe it just introduces confusion in understanding and using a smart contract. I mean would would renaming the one function to safeTransferFromCallData or safeTransferFromWithData be such a big problem?
We could perhaps have a need for strict mode by default, that you can disable it via a pragma tag?

[jacqueswww]

@maurelian on a side note. Started reading that spec, it's even more confusing as to why there is a safeTransferFrom and a transferFrom ? I would assume one would only allow the safe kind :stuck_out_tongue:

[fubuloubu]

I do agree that overloading is confusing and reduces readability.

I guess the answer to this question is a political one: do we refuse to support less readable standard proposals (or optional behaviors like this)

Or do we allow it by requiring a compiler flag? I'm not sure I like littering code with pragmas, a compiler flag would do just as well turning errors of experimental features into warnings to allow it to compile.

[ben-kaufman]

Just reminding we already discussed that in regards to ERC827 and agreed the standard should be changed. I don’t think it’s such an issue because there is no approved EIP with function overloading afaik.
I think it’s good to keep the discussion open but in my opinion the problems overloading may cause are bigger than its benefits.
I think changing these draft stage standards to use clear explicit names instead of confusing overloading would be much better for both Solidity and Vyper.

[fubuloubu]

Thanks @ben-kaufman for summing that up very well!

[fulldecent]

Here is a separate proposed specification that I believe is compatible with Vyper values.

MOTIVATION:

• Vyper is currently incompatible with certain EVM features. This is a deficiency.
• A motivation for disallowing function overloading is listed in README.md as "Another problem with function overloading is that it makes the code much harder to search through as you have to keep track on which call refers to which function."

SPECIFICATION:

1. Allow a new syntax for defining functions:

def transfer_stock{3}(receiver: address, transfer_order: uint256(currency_value)):
def transfer_stock{4}(receiver: address, transfer_order: uint256(currency_value), something: bool):

The additional {x} note is a insignificant from the code-generation perspective.

1. Require the note for function calls:

    self.transfer_stock{4}(an_address, a_value, a_bool)
    self.transfer_stock{3}(an_address, a_value)

1. It is a compiler error to have two functions with the same name + note

2. Optional: throw a warning or error if function notes are used for non-overloaded functions

DISCUSSION

In this way, it is now simple to search through to find which call refers to which function. This resolves the original motivation for disallowing function overloading.

[jacqueswww]

Vyper is currently incompatible with certain EVM features. This is a deficiency.

I vehemently disagree with statement, the ABI allows function overloading - yes. The EVM doesn't define any rules with regards to the ABI. But this doesn't mean it should be encouraged.
It's vyper job, to make maximally difficult for a programmer to write misleading code, and I think it's quite clear that function overloading supports a devious programmer in doing so.

In web3.js it's also extra work / and actually quite difficult to consume overloaded functions.

[jacqueswww]

As discussed on gitter if we end up having to support function overloading I suggest the following:

1.) We only allow overloaded functions to be grouped together when defined, ie. they have to follow each other.
2.) We add an override decorator.

@allow_overload
@public
def safeTransfer():
    pass

[fulldecent]

Hello everyone, please see #903.

That considers only a limited scope of function overloads. I think that is a feature that Vyper ACTUALLY wants, not just something to be begrudgingly accepted.

[jacqueswww]

As #903 has been merged, I am also closing this issue ;)