Compiler warnings after instrumentation

[aleybovich]

I have the following library:

pragma solidity ^0.5.1;

library SafeMath {
    function add(uint256 a, uint256 b) internal pure returns (uint256) {
        uint256 c = a + b;
        require(c >= a, "SafeMath: addition overflow");
        return c;
    }

    function sub(uint256 a, uint256 b) internal pure returns (uint256) {
        return sub(a, b, "SafeMath: subtraction overflow");
    }

    function sub(uint256 a, uint256 b, string memory errorMessage) internal pure returns (uint256) {
        require(b <= a, errorMessage);
        uint256 c = a - b;
        return c;
    }

    function mul(uint256 a, uint256 b) internal pure returns (uint256) {
       if (a == 0) {
            return 0;
        }

        uint256 c = a * b;
        require(c / a == b, "SafeMath: multiplication overflow");
        return c;
    }

    function div(uint256 a, uint256 b) internal pure returns (uint256) {
        return div(a, b, "SafeMath: division by zero");
    }

    function div(uint256 a, uint256 b, string memory errorMessage) internal pure returns (uint256) {
        // Solidity only automatically asserts when dividing by 0
        require(b > 0, errorMessage);
        uint256 c = a / b;
        // assert(a == b * c + a % b); // There is no case in which this doesn't hold

        return c;
    }

    function mod(uint256 a, uint256 b) internal pure returns (uint256) {
        return mod(a, b, "SafeMath: modulo by zero");
    }

    function mod(uint256 a, uint256 b, string memory errorMessage) internal pure returns (uint256) {
        require(b != 0, errorMessage);
        return a % b;
    }
}

When I run solidity-coverage, the first compilation has no warnings, then goes instrumentation and the second round of compilation produces the following warnings:

/home/andrey/dev/path-protocol/coverageEnv/contracts/SafeMath.sol:26:5: Warning: Function state mutability can be restricted to pure
    function add(uint256 a, uint256 b) internal      returns (uint256) {
    ^ (Relevant source part starts here and spans across multiple lines).
,/home/andrey/dev/path-protocol/coverageEnv/contracts/SafeMath.sol:55:5: Warning: Function state mutability can be restricted to pure
    function sub(uint256 a, uint256 b, string memory errorMessage) internal      returns (uint256) {
    ^ (Relevant source part starts here and spans across multiple lines).
,/home/andrey/dev/path-protocol/coverageEnv/contracts/SafeMath.sol:71:5: Warning: Function state mutability can be restricted to pure
    function mul(uint256 a, uint256 b) internal      returns (uint256) {
    ^ (Relevant source part starts here and spans across multiple lines).
,/home/andrey/dev/path-protocol/coverageEnv/contracts/SafeMath.sol:111:5: Warning: Function state mutability can be restricted to pure
    function div(uint256 a, uint256 b, string memory errorMessage) internal      returns (uint256) {
    ^ (Relevant source part starts here and spans across multiple lines).
,/home/andrey/dev/path-protocol/coverageEnv/contracts/SafeMath.sol:146:5: Warning: Function state mutability can be restricted to pure
    function mod(uint256 a, uint256 b, string memory errorMessage) internal      returns (uint256) {
    ^ (Relevant source part starts here and spans across multiple lines).

Looks like instrumentation strips pure for those library methods. It's not a huge deal in this case as it doesnt affect test results or coverage, but it just feels like a bug

[cgewecke]

@aleybovich It's not a bug - #146 contains a long discussion of this subject if you're interested...

The coverage tool works by injecting event statements into the contracts but Solidity does not allow these in a view or pure function since v0.5.0. To get around this restriction we compile twice - first to get the 'correct' ABI (with view/pure). Then instrumented and without state-mutability modifiers to get the bytecode 'with coverage'. At the end we swap the original ABI back into the artifact which tricks truffle into invoking those methods as .call rather than .send and everything works out.

In 0.7.0 we're going to move to a simpler strategy though which won't change mutability modifiers.

[aleybovich]

@cgewecke thank you for the explanation