I’d like to get some feedback about specific compiler warnings that people often ignore (or would like to ignore) and reasons behind it.
Do you have some specific examples of such warnings produced by the latest compiler version? (preferably with code snippets)
Why don’t you just fix the warnings?
Are they in the code you don’t have control over?
Are they just false-positives? Maybe just a compiler bug?
Do you simply disagree with them?
Any other reasons?
I hope this will let us address the most problematic cases. It will also give us some input to consider whether we actually need a feature that allows disabling specific warnings. Such a feature might be convenient in individual cases but if it’s really just a half-measure to hide the symptoms and we’d rather address the cause of such problems.
Warning: Variable is shadowed in inline assembly by an instruction of the same name
I want to be able to have a SafeMath library with an add function AND use inline assembly. This is so common I’m not giving notice anymore… To a point that I’m not sure it protects me at all.
Warning: Failure condition of ‘send’ ignored. Consider using ‘transfer’ instead.
I always ignore these. Compiler thinks that transfer() is always a better alternative to send() but it’s not the case actually. In some cases, receiver of the funds is not a trusted party, and this party can make the transfer() call fail on purpose to prevent the smart contract functioning properly (a.k.a. denial of service attack). So in some cases, we don’t want the transaction revert even if beneficiary can’t receive funds.
Example scenario: A function that distributes rewards to users. It loops over a set of users, one of them is malicious and rejects the payment to make the transaction fail. Because of this nobody can receive the reward.
In general I would really encourage the solidity team to add a feature that allows warnings to be disabled (at least on a per file level). Having to wait for a new compiler version to squash a superflous warning is very frustrating and will not help code that must compile against older versions. In general I would prefer to always have every warning treated as an error, and then disable those that do not apply to my specific context. This workflow is simply impossible in solidity, especially if these warnings are triggered when compiling older code or dependencies that I cannot change (e.g. importing uniswap or maker as a test fixture).
Regarding specific warnings, I would say that a good chunk of all of the warning clutter I get comes from the following two checks:
Max Code Size Exceeded
In dapptools unit tests are written in solidity. The test contracts always exceed the allowable size for mainnet code, but it doesn’t matter because they’re only ever run in hevm which doesn’t have a maximum size for code. It would be awesome to be able to selectively supress this warning for certain contracts (the check itself is very useful for code that will actually end up being deployed of course)
Constructor Visibility Ignored
It is possible to write code that is compatible across many different compiler versions (e.g. 0.6 → 0.8), and sometimes this is an extremely useful thing to do (e.g. ds-test, which contains all the solidity test helpers used in dapptools tests, or weird-erc20, which provides various erc20 related test fixtures). It is however not possible to write this code without triggering this warning, resulting in a horribly cluttered compiler output. In general deprecation warnings are a good practice, but I would really like to be able to disable this warning for code that needs to work across many compiler versions.