Modifier arguments should allow ExpressionStatements

raineorshine commented 7 years ago

This fails to parse:

pragma solidity ^0.4.4;

contract A {
    function yes() public constant returns(bool) {
        return true;
    }
}

contract MyContract {
  A a;
  modifier only(bool b) { _; }
  function foo() only(a.yes()) {
  }
}

If a.yes() is replaced with true it parses fine.

tcoulter commented 7 years ago

Interesting find. Solidity allows arbitrary expressions and function calls in modifiers?

raineorshine commented 7 years ago

Yes. Apparently I am one of the few who actively uses them :D.

I'm not 100% skilled with PegJS, but I have a rough draft of a fix. About to create a PR.

tcoulter commented 7 years ago

Your PR looked good. I added a comment about ensuring the very simplistic tests pass. Note that one of the tests tests the built parser to ensure we're always rebuilding a new version as we add features. So feel free to commit a new built version if you like.

raineorshine commented 7 years ago

Okay, good to know. I saw federico's PR didn't have a new build so I wasn't sure whether it was expected or not. On Mon, Nov 21, 2016 at 4:18 PM Tim Coulter notifications@github.com wrote:

Your PR looked good. I added a comment about ensuring the very simplistic tests pass. Note that one of the tests tests the built parser to ensure we're always rebuilding a new version as we add features. So feel free to commit a new built version if you like.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ConsenSys/solidity-parser/issues/27#issuecomment-262098586, or mute the thread https://github.com/notifications/unsubscribe-auth/AAtyxHvFAyabetJP8_LZ_T1KksaHiBpIks5rAiaqgaJpZM4K4vSK .

ConsenSysMesh / solidity-parser

Modifier arguments should allow ExpressionStatements #27