Function overloading leads to duplicate symbols
Since in ink! constructors also have names. This is another precondition for making #989 happen:
- Allow to give constructors a name. We need a name for
ink_metadata. We can always fallback tonewbut it would make a nicer user experience to give an actual name for the constructor. - Disallows constructor overloading for Substrate. Reasons see my below comment.
Support for constructor renaming with overloading can be implemented later on (once the Substrate target is working again for the latest runtime), This could then also be done for Solana.
EDIT:
This issue exists not just for constructors. We discussed about disallowing overloading function and constructors within the same Contract for Solana and Substrate. One problem is (for example for Substrate) that all functions must have a unique name in the metadata. Otherwise, we can 1. not reuse the ink metadata crates for ABI generation and 2. our contracts ABI would not be compatible with the official tooling (TypeScript libraries for example). We could see 2 solutions:
- Disallow overloading within the same Contract
- Mangle the function names for overloaded functions
Solution 2 would be a hack. How would you know that a functions name was mangled? It doesn't seem right to only mangle some function names and not others. But having function name mangling for all functions and constructors would result in ugly symbols (function names) everywhere. Solution 1 would have the drawback that we disallow a Solidity language feature. But it would be the much cleaner solution.
I discussed this with @seanyoung today, since implementing this might be more involved than I initially thought. We concluded the following:
Complexity stems mostly from having named arguments and having the ability to overload constructors together with the ability to name them. In ETH Solidity, neither overloading nor naming constructors is possible and it would require some more careful design decisions and possibly require some refactoring within affected parts in solang.
To make progress towards having a fully functional and up-to date Substrate target again as soon as possible, we decided to just
disallow constructor overloading for the substrate target for now. If a constructor then has no name, we can simply name it new. Since constructor overloading is not at all possible in ETH solidity anyways this solution should make for a fair compromise. Additionally it can still be fixed in the future.
WTF thanks DCO
This is getting awkward with the history. I squashed and opened #1035 instead