_quorumNumerator may not be initialized in GovernorVotesQuorumFraction.quorumNumerator() and quorumNumerator(uint256 blockNumber)

[kristovatlas]

Nitpick: _quorumNumerator is not initialized in GovernorVotesQuorumFraction and if not set first will default to Solidity's default integer value (currently zero). Consider setting the value explicitly to 0 in order to make the value properly documented. (Note: the variable has been marked as "deprecated" in a code comment, so alternatively just delete it and referencing functions if deprecated.)

💻 Environment

Current master branch

🔢 Code to reproduce bug

N/A

[Amxx]

Hello @kristovatlas

the variable has been marked as "deprecated" in a code comment, so alternatively just delete it and referencing functions if deprecated

That is something we can't do.

You noticed that we deprecated it in favor of Checkpoints.History private _quorumNumeratorHistory; which means we no longer write to it (we put a checkpoint in the history instead). We only read it if the history length is 0, which will never happen* has the constructor pushed a checkpoint.

Why do we keep it declared, and why do we have a _quorumNumeratorHistory._checkpoints.length == 0 check ??? For the upgradeable version of the contract !!!

The upgradeable contracts are automatically generated from this non-upgradeable code. For the upgradeable version to remain compatible, we need to keep the storage layout. This is why we don't remove the slot declaration. Also, if an upgradeable contract transitions from the previous implementation to this, the constructor is never run, and no initial checkpoint is inserted into the history. That is why we have the fallback, to read the old slot if the new structure is not initialized.

I hope this clarify the reason why we keep the variable, but don't write to it, and don't bother setting it.