Disallow internal function pointers in storage

[cameel]

Related: #15716, #15721.

Abstract

Disallow using internal function types for storage variables.

Motivation

Compiler's guarantees about the stability of internal function pointers are pretty weak. Such pointers are only valid in the context of a single contract and even then only in a very strict sense - any bytecode change may invalidate them.

Such types are already considered internal and cannot be passed across contracts (unless masked as something else). Putting them in storage can be safe if done carefully but the fact that it has to be considered at all is not obvious to most users and a pretty big gotcha.

We recently asked for community feedback in [Deprecation feedback] Disallowing internal function pointers in storage and the answers we received so far seem in favor of the change.

Specification

• The use of an internal function type (function() internal and its derivatives) should be disallowed for all variables stored in storage and references to storage variables, which includes:
  • contract state variables
  • storage function parameters/returns
  • storage local variables
  • members of composite types (structs, arrays, mappings)
• Such types should still be allowed in other locations, including transient storage and immutables.

Backwards Compatibility

The change is breaking. Contracts using such pointers will no longer compile.

Upgrading an existing contract while keeping the storage layout will still be possible - by replacing the storage variable with an integer and creating a custom dispatch function. The dispatch would simply treat the value as an enumeration and call the internal function corresponding to each value. It will also be possible via inline assembly, though such usage will require referencing any internal function that might be called from outside inline assembly to prevent the compiler from considering it unused and removing it.

[cameel]

An additional design question is whether we should drop the special layout for such pointers in transient storage.

Internal functions that exist at creation time are completely distinct from those that exist in deployed bytecode. The compiler abstracts this away by duplicating function's code if it is referred to from both. Since pointers to such functions can be stored at creation time and used at deployment time, they are a combination of two separate addresses.

Now we could drop that behavior, which would simplify the compiler a little.

On the other hand, we probably still have to keep that double layout for immutables because those can be used in both contexts. Also, IIRC we no longer do this in IR codegen (we store abstract IDs, not jump addresses) so this would only simplify legacy codegen anyway.

[cameel]

Note also that the fact that pointers can be passed from creation to deployed context means that all internal functions whose pointers are used at creation time have to be duplicated in deployed code. That's even if they're never actually called by deployed code, because the compiler can't determine that.

Due to the fact that we'll still support immutables, this deprecation does not let us get rid of this inefficiency.