Should overriding function declarations require the `override` keyword?

Language Design
1

Hello,

I’d like to ask for more feedback on one of the issues from the bug tracker: Do not require override for functions from interfaces #8281. We’d like to make a decision there and either move it to implementation backlog or close it but the current discussion is inconclusive so far and all the options have their proponents.

Let me summarize the available options first:

  1. Do not change anything. Keep requiring override no matter whether the function being overridden is only declared in an interface or defined in a contract.
  2. Do not require override when overriding a function that is only a declaration.
    • variant A: Do not require it even if declarations from multiple base contracts/interfaces are being overridden (i.e. the override(Base1, Base2) syntax).
    • variant B: Still require it when overriding declarations from multiple base contracts.
  3. Introduce a new keyword (e.g. implements) to be used when only a declaration is being overridden.
    • variant A: If multiple inheritance is used and some of the base contracts have a definition and some have a declaration, require both override and implements.
    • variant B: Do not require implements if override is already present.

Please say which option you would prefer and why.

Two additional questions also require clarification. I think that the answer should be “yes” in both cases for consistency but I want to make sure everyone agrees:

  1. Should the same rules apply to functions declared in interfaces and functions without a body in abstract contracts?
  2. Should the same rules apply to virtual modifiers without a body?

Below is my attempt at summarizing arguments from the issue with some extra additions from me. All organized based on use cases of the override keyword.

Use case 1: Ensuring that the overridden function exists

  • Without any keyword the compiler cannot detect mistakes. Removing it removes the check to ensure that the function actually exists in the interface and that it has the same name, parameters and return values.
    • In many cases this check is redundant because if there is a mismatch, the compiler will notice that not all functions from the interface have been implemented.
    • There are cases where it’s not redundant. For example when a new implementation is being added in an abstract contract.
  • override allows detecting mismatches if the interface gets modified. The compiler can point out all the functions in derived contracts that need to be updated too.
    • This, again, can be redundant in many cases.
    • A related situation is when you switch from one base class implementing an interface to another that’s supposed to be a drop-in replacement. With a keyword the compiler can notice that the new base class has an extra declaration that matches one of the functions in the derived class (see @ekpyron’s example).
  • Removing the keyword also removes this check for situations when a declaration in one interface overrides a declaration in another.
    • Note that when an interface inherits from multiple other interfaces providing the same declaration, it must override it so it’s not a useless in practice as it may seem at first.

Use case 2: Clarifying that the inherited behavior might be affected

  • Behavior can only change if an implementation already exists. Declarations in interfaces do not define behavior so having them also covered by override reduces the usefulness of the keyword in this use case.
    • This is especially important for auditing.
  • Overriding functions from interfaces is very common and requiring override desensitizes the reader to it. It no longer stands out as something worth paying attention to.
  • Adding the keyword to nearly all functions is also tedious when writing code.

Use case 3: Clarifying which function is being overridden

When multiple virtual functions with the same signature are defined independently in multiple base contracts, the compiler requires specifying the names of these contracts to make sure the programmer is aware that they are all being overridden. I think this is not a big concern when the functions being overridden are only declarations.

Use case 4: Ensuring that an interface is completely implemented

The compiler can already detect that not all declaration from an interface or an abstract contract are implemented. Removing the keyword for overrides of declarations does not change that.

2

Hi,

Thanks very much for this considered and thorough treatment.

Consistency

Most likley controversial feedback first: I see interfaces and abstract contracts as serving two different use cases. The safety offered by override seems to come at the cost of making simple uses of interfaces less apealing. And trying to treat them as the same as abstract methods is contributing to that effect.

Use case 1.

I don’t understand how both “Without any keyword the compiler cannot detect mistakes” and “In many cases this check is redundant because if there is a mismatch, the compiler will notice that not all functions from the interface have been implemented” can both be true. What is going on there ?

I am in favour of option 2 variant A. On the grounds that:

a) It feels like a big compromise for a rare case. A case I think likely only a real problem in complicated inheritance graphs. The silent override of ‘validate’ ilustated by @ekpyron I think only applies if abstract contracts are in the graph ? @ekpyron suggested here that scenarios where override saved the day were possible without mixing abstract and interface. I don’t have the solidity depth to challenge that.

b) It diminishes the significance (to the programmer) of the override keyword - pointed out by @nventuro here. I strongly agree with this point.

c) Isn’t a linter a better place to catch this sort of thing ?

d) In our consortia private deployment scenario this is a complete non issue. I only make this point to acknowlege that our uses are not remotely comparible to contracts on public eth.

I would prefer a class level opt in. When I write B in ekpyron’s example and I’m choosing to inherit from sophisticated 3rd part contracts with serious audit behind them I declare my contract to require these controls. I appreciate that bifuricates the compiler paths and testing and validation and that is not free, but it seems like what I would want. Even so, the current implementation of override applied in this way would still suffer from the signal/noise problem.

Use case 2 yes, finding a solution that doesn’t impact interfaces in this way would be great!

Use case 3. This just seems to require the programmer to both inspect the details of all the contracts they inherit from (fine I do that anyway), and then convince the compiler they have done so. I don’t want to do the second bit. Typing the names of the functions I am overriding doesn’t guarantee (or even softly imply) I have understood what I have looked at. Assertion: Everybody presses ‘y’ without thinking all the time.

Use case 4. yes, the thing I find most valuable about interfaces is not helped by override

Again, thanks very much for continuing to consider this issue with care.

Robin

3

I don’t understand how both “Without any keyword the compiler cannot detect mistakes” and “In many cases this check is redundant because if there is a mismatch, the compiler will notice that not all functions from the interface have been implemented” can both be true. What is going on there ?

What I meant is that without override the compiler cannot tell you if you are defining a new function or overriding one from an interface so if the name does not match you won’t get the error that your function does not really override anything. You will get another error though - that there is a function without definition. Still, you will not get it in all cases. For example if your contract is abstract the compiler does not require all functions to be implemented.

4

My preference is for option 2A. I would sum it up by saying abstract function declarations should not “count” towards override logic.

Any mistakes that I can think of would result in an abstract contract or one that doesn’t compile. If the contract is meant to be abstract, the unimplemented function would go undetected if the abstract contract is compiled on its own, but this is very likely never the case. Even if someone is building a library of abstract contracts, they would have to write concrete contracts in order to test them and this would not be possible if there is an abstract function that was not implemented.

I would consider additionally option 3B but where the implements keyword is fully optional (without even a warning). The language is already too verbose and I don’t think adding more keywords and mandatory syntax would be good, but some people may choose to opt in to this additional check.

I think this was implied in the post but I’d say this change should be done backwards compatibly, so only lifting the requirement to include override while not disallowing it, and not producing any warnings when it is included.

Regarding interfaces vs abstract contracts, I initially felt that they should be treated differently, but I’m now convinced that they should be the same, by the logic that an override keyword should imply that if super is not called there is a piece of code that is being ignored.