Allow private visibility for modifier keyword

[fulldecent]

pragma solidity ^0.4.19;

contract A {
    modifier mod private {
        require(false);
        _;
    }
}

contract B is A {
    function B() mod public {
        
    }
}

In this example, contract A uses a modifier mod for its own purposes (that you can't see here today).

Contract A would like to avoid exporting this modifier for inherited contracts. But unfortunately, B's environment has become polluted by this unwanted side effect.

---

Work plan

• Allow modifier to have visibility specified as internal or private, other visibilities shall be an error
• If not specified, the modifier's visibility is implicitly internal
• Update language grammar documentation
• Handle visibility inheritance the way inheritance is supposed to be handled (see test cases), not the way Solidity currently handles visibility inheritance
• Update modifier documentation

Test suite

pragma solidity ^0.4.19;

contract Test1 {
    modifier mod private {
        require(false);
        _;
    }
}

// /////////////////////////////////////////////////////////////////////////////

contract Test2 {
    modifier mod internal {
        require(false);
        _;
    }
}

// /////////////////////////////////////////////////////////////////////////////

contract Test3 {
    modifier mod {
        require(false);
        _;
    }
}

// /////////////////////////////////////////////////////////////////////////////

contract Test4A {
    modifier mod private {
        require(false);
        _;
    }
}

contract Test4B is Test4A {
    modifier mod private { // <--------- No error, this now has scope
        require(false);
        _;
    }
}

// /////////////////////////////////////////////////////////////////////////////

contract Test5A {
    modifier mod private {
        require(false);
        _;
    }
}

contract Test5B is Test5A {
    modifier mod internal { // <--------- No error, this now has scope
        require(false);
        _;
    }
}

// /////////////////////////////////////////////////////////////////////////////

contract Test6A {
    modifier mod internal {
        require(false);
        _;
    }
}

contract Test6B is Test6A {
    modifier mod private { // <--------- Error: this is more restrictive than base contract
        require(false);
        _;
    }
}

// /////////////////////////////////////////////////////////////////////////////

contract Test7A {
    modifier mod internal {
        require(false);
        _;
    }
}

contract Test7B is Test7A {
    modifier mod internal { // <--------- No error, this now has scope
        require(false);
        _;
    }
}

// /////////////////////////////////////////////////////////////////////////////

contract Test8A {
    modifier mod private {
        require(false);
        _;
    }
}

contract Test8B is Test8A {
    function Test8B() mod public { // <-------- Error, modifier mod is not visible here
    }
}

[chriseth]

Can you explain "Handle visibility inheritance the way inheritance is supposed to be handled (see test cases), not the way Solidity currently handles visibility inheritance" in a bit more detail, please?

There is also the option to disallow overriding for modifiers in general, which would probably be a good idea. Currently, modifiers are virtual, but I'm not sure if people actually use that. If you really need that, you can always call a virtual function inside the modifier.

[fulldecent]

Regarding visibility inheritance -- currently you cannot override functions with a function of different visibility (can't find issue), but in the future Solidity will allow greater than / equal visibility in subclasses.

These test cases demonstrate the LATTER, which is inconsistent with how functions currently work, FORMER.

---

Woah, right now modifiers are dynamic dispatch? Wow, TIL B.x and B.y both return false.

pragma solidity ^0.4.19;

contract A {
    modifier m {
        require(true);
        _;
    }

    function x() m pure external {
    }
}

contract B is A {
    modifier m {
        require(false);
        _;
    }
    
    function y() m pure external {
    }
}

Ok, well if modifiers are virtual then I would expect this to work but it doesn't.

pragma solidity ^0.4.19;

contract A {
    modifier m;
}

---

Wow, TIL, private functions can also be overridden with virtual dispatch.

---

Ok, well today I learned a lot more about how Solidity actually works. If you read the documentation before writing code you might think the language is messed up (read documentation on contract function inheritance versus the above). But if you just start writing code and seeing how it works then it makes sense how things work.

In that case, here is the way that interfaces can be fixed to be more Solidity-like as per the way that Solidity actually works:

• Implicit internal visibility
• Explicit internal or private visibility
• Dynamic dispatch, just like functions, even for private modifiers
• You can't override a modifier unless your override has the same visibility (change this later, as part of that other issue I can't find which addresses the same situation for functions)
• Allow defining a modifier without implementation, then implement in the inherited contract

[fulldecent]

Closing. Do not have a concrete need. 0.6.0 has virtual and override.