My suggestion would be to go further and disallow declaring storage variables of internal function types. The question is whether anyone thinks this is a bad idea or would be negatively impacted by this? If there are no objections, we’re likely to go ahead with this idea for 0.9.0.
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 it also seems like the fact that it has to be considered at all is not obvious to most users and a pretty big gotcha. It will still be possible via inline assembly of course but that will at least make it clearer that something unusual is happening.
Definitely agree with deprecating and removing this. It shouldn’t cause any issue, it can be recovered manually if someone needs it, and that’s the way it should’ve been originally.
Would this impact immutable function pointers that are set during contract construction? I’ve found those to be handy when you different ways that something could be handled depending on how a contract is initialized.
Immutables seem perfectly safe because they are stored in the contract code so they would be updated along with it. We’d let those be and only disallow putting internal pointers in storage.
Does anyone think we should disallow them in transient storage as well? On the one hand it’s technically possible to update a contract during a transaction, so it has the same problem as storage. On the other, the same could be said about memory and disallowing such pointers in memory seems to me like it would be going too far. So maybe transient storage is fine.
In any case, thanks for pointing out immutables. I actually didn’t think about that use case.