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

Clean up inheritance and overriding specification #3698

Closed
chriseth opened this issue Mar 9, 2018 · 10 comments
Closed

Clean up inheritance and overriding specification #3698

chriseth opened this issue Mar 9, 2018 · 10 comments

Comments

@chriseth
Copy link
Contributor

chriseth commented Mar 9, 2018

@fulldecent if you want, you can draft a specification here.

@chriseth
Copy link
Contributor Author

chriseth commented Mar 13, 2018

I think we should have the following minimum requirements:

Functions being present in super contracts cannot somehow disappear through adding a sub contract.

This means, the visibility can for example change from external to public, but not from public to external or from external to internal.

I would say that any other such change is allowed. It is also allowed to restrict the state mutability, but not to relax it.

If a function overrides a function from a super contract, it needs the override specifier, everything else is treated as an error.

An external function (but not a function of any other visibility) can be overridden by a public state variable. This means that the public state variable also needs the 'override' keyword.

It is an error for two contracts in an inheritance hierarchy to have a member of the same name, unless one member (f_B) is defined in a class (B) that derives (directly or indirectly) from the class (A) the other member (f_A) is defined in; and: f_B uses the override keyword, f_A is a function and f_B is either a function or a public state variable, both are compatible with regards to visibility and state mutability as per the above rules and both have exactly the same arguments.

This in particular means that private members are not hidden from this mechanism and that overriding and overloading is incompatible.

Furthermore, it is disallowed to inherit members of the same name that come from different base contracts unless they share a common base contract that defines the function to be overridden.

The above does not yet capture modifiers.

TODO:

  • talk about overriding modifiers
  • shall we make overriding and overloading work at the same time?
  • shall we allow name clashes with private members?
  • what about functions that have different return types?

@chriseth
Copy link
Contributor Author

chriseth commented Mar 13, 2018

Interfaces should be allowed to inherit from interfaces.

@fulldecent
Copy link
Contributor

fulldecent commented Mar 13, 2018

My understanding is that

Furthermore, it is disallowed to inherit members of the same name that come from different base contracts unless they share a common base contract that defines the function to be overridden.

logically follows from

It is an error for two contracts in an inheritance hierarchy to have a member of the same name, unless one member (f_B) is defined in a class (B) that derives (directly or indirectly) from the class (A) the other member (f_A) is defined in; and: f_B uses the override keyword, f_A is a function and f_B is either a function or a public state variable, both are compatible with regards to visibility and state mutability as per the above rules and both have exactly the same arguments.

and does on not provide any further prescription.


Just to clarify, currently all functions are called with dynamic dispatch.


Regarding private functions, I am not sure what the best solution is. Currently we have overrideable with dynamic dispatch.

pragma solidity ^0.4.16;

interface HasAName {
    function name() external pure returns (string);
}

contract Animal {
    function toString() private pure returns (string) {
        return "animal";
    }
    
    function name() external pure returns (string) {
        return toString();
    }
    
    function favoriteCrackers() external pure returns (string) {
        return /*concat(*/ toString() /*," crackers")*/;
    }
}

contract Dog is Animal {
    function toString() private pure returns (string) {
        return "dog";
    }
    
    function name() external pure returns (string) {
        return toString();
    }
}

Another option is no overrides and no dynamic dispatch. As in local declaration. So then the dog's favorite crackers would be animal crackers.

The animal crackers solution makes a lot of sense too. This would solve the problem of private name clashes (they are allowed).


If you can override modifiers, then virtual modifiers should be allowed.


Can you please link to the overloading proposal? Yes, overloading is orthogonal to overriding. Because the fully qualified name for functions is the function selector.


A function that returns a different value cannot implement a parent function, this is an error. Exception (actually a note): if animal crackers then private functions don't count as overriding and therefore different returns are allowed.

@fulldecent
Copy link
Contributor

We should specify that natspec is inherited.

pragma solidity ^0.4.21;

import "http://github.com/fulldecent/su-squares-bounty/contracts/ERC165.sol";

/// @title A reusable contract to comply with ERC-165
/// @author William Entriken (https://phor.net)
contract PublishInterfaces is ERC165 {
    /// @dev You must not set element 0xffffffff to true
    mapping(bytes4 => bool) internal supportedInterfaces;

    function PublishInterfaces() internal {
        supportedInterfaces[0x01ffc9a7] = true; // ERC165
    }

    function supportsInterface(bytes4 interfaceID) external view returns (bool) {
        return supportedInterfaces[interfaceID];
    }
}

Here, the supportsInterface should get the natspec.

@OTTTO
Copy link

OTTTO commented Mar 14, 2018

Interfaces should allow enum definitions.

interface Interface {
    enum EnumType { Option, OtherOption } 
    function initiateProvider(bytes32, EnumType, uint256, uint256);
    function getProvider(address, bytes32) view returns (EnumType, uint256, uint256);
}

@chriseth
Copy link
Contributor Author

Ok, I already see that this is not the right way to discuss :)
I will create pull request to the documentation.

@pirapira
Copy link
Member

@OTTTO that idea is for a separate GitHub issue.

@chriseth
Copy link
Contributor Author

Ok, I created a pull request #3729 - let's continue discussing there. @OTTTO I included your suggestion in the PR.

@fulldecent I think I addressed all your comments except for the one about private functions. Are private functions actually used that much? If we allow private functions to be "re-defined" in derived contracts, it might lead to confusion. I also think that the use-cases for private functions are limited. If we just disallow overriding private functions, it turns them into some kind of final functions.

@chriseth
Copy link
Contributor Author

Closing this, please continue the discussion on #3729

@fulldecent
Copy link
Contributor

I have reviewed all my notes here and they are handled or reiterated in the new PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants