Add info for virtual and override functions

[fvictorio]

FunctionDefinitions nodes with the virtual or override keywords cannot be distinguished from nodes that don't have if. In other words:

function foo() public virtual returns (uint);

and

function foo() public returns (uint);

are represented by the same node.

I don't know the best way to add this. For virtual, maybe just a isVirtual property would be enough, and consistent with how it's being done (isConstructor, isFallback, etc.).

For override, since this can have parameters, I think a (possibly null) override or overrideSpecifier would work.

[GNSPS]

This is now partially addressed with 0d9bf26

Will implement the override node in the coming days

[fvictorio]

Awesome! Feel free to add me as a reviewer if you want feedback on some PR.

[fvictorio]

Hey @GNSPS, I would like to give this a try. Do you have some pointers to give me? Is this something that can be done without changing the grammar?

[GNSPS]

Hey @fvictorio! I was at ETHDenver and left my email pile up, sorry about that!

Yes, it should work without changing the grammar.

I would start by checking this out:

https://github.com/ConsenSys/solidity-parser-antlr/blob/master/src/ASTBuilder.js#L252-L278

And, more specifically the example of the virtual keyword here:

https://github.com/ConsenSys/solidity-parser-antlr/blob/master/src/ASTBuilder.js#L271-L274

[nicholaspai]

Hey @GNSPS @fvictorio I tried putting in a fix for this a local fork but couldn't quite figure out how the codes work, particularly in the ast arrays in src/lib/Solidity.interp and src/lib/SolidityLexer.interp and the majority of the codes in src/lib/SolidityParser.js

[fvictorio]

@nicholaspai that files are automatically generated by ANTLR, they are not meant to be read.

I haven't worked a lot on the parser, but the way I go about this is:

• Check the grammar in the included (submodule) project: https://github.com/ConsenSys/solidity-antlr4/blob/bf22351c47b65b1d1d705ede26c80643294020aa/Solidity.g4
• Check the ASTBuilder file in this project: https://github.com/ConsenSys/solidity-parser-antlr/blob/master/src/ASTBuilder.js

[nicholaspai]

@fvictorio it looks like there is already an overrideSpecifier in the grammar, but it doesn't look like its in the ASTBuilder

modifierDefinition
  : 'modifier' identifier parameterList? ( VirtualKeyword | overrideSpecifier )* block ;

[fvictorio]

Yes, I think that's the case. I will open a PR with some failing tests. If @GNSPS is ok with the API proposed there, then that can be the base for working on this.

[fvictorio]

See #9