Open fvictorio opened 4 years ago
This is now partially addressed with https://github.com/ConsenSys/solidity-parser-antlr/commit/0d9bf261ef3d0df1e035869760beb7a8bf0d0c2f
Will implement the override node in the coming days
Awesome! Feel free to add me as a reviewer if you want feedback on some PR.
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?
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
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
@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:
@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 ;
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.
FunctionDefinition
s nodes with thevirtual
oroverride
keywords cannot be distinguished from nodes that don't have if. In other words:and
are represented by the same node.
I don't know the best way to add this. For
virtual
, maybe just aisVirtual
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
oroverrideSpecifier
would work.