import ast from JSON

[djudjuu]

This introduces a new input format to the compiler: an AST in JSON format, as produced by --standard-json`` or ```` --combined-json ast,compact-format. Importing an AST leaves the compiler in the same state as after CompilerStack::parse().

Motiviation: This could enable things like mutation testing.

Current status:

• --import-ast option in commandline able to import and compile all kinds of AST-nodes

• this assumes that inline assembly is exported to an AST as well, so inline assembly to AST in json export #7537 will have to be merged first.

• testuite that verifies

  • that an imported and exported AST is the same as original import

The testsuite runs on all contracts defined in /test/libsolidity/syntaxTests/. As some syntaxTestfiles define multiple sources in one file, a script is added that separates those to their own files (creating directories in the process) and removing those after it is done.

Other changes introduces by this PR

• in the metadata the compiler now
  • defines new input-language solidityAST, and
  • [sources][sourceName][content] is the compactPrint of the Json used as source (needs testing)
• Also, the PseudoAstNodes, such as created when parsing magic variable expressions such as this, block.x, msg.x now all have the special nodeID < 0
• • When resetting the IDDispenser, there now is the option to register dispensed Id's (and throw errors if an an AST uses the same Id more than once.
• ASTJson-export now creates full Identifier-ASTNodes for the original contractIdentifier in importStatements such as import {C as BC} from .... Previously C was only referred to with its ID and the information about the name C was lost, which made it impossible to reconstruct the IdentifierNode.
• IDDispenser can now keep track of dispensed IDs if asked to do so during reset.

left TODOs

This PR assumes that InlineAssembly is exported to an ast (currently being reviewed in
#7537).
So to make it work and develop the tests I added the changes from there in a second commit.

• So before this would be entirely merged, one should remove the second commit in this PR.
• add scripts/AstImportTest.sh to the testsuite. I tried adding it to cmdlineTests.sh which worked for me locally, but it broke some tests.
• remove (currently commented out) debugging echo xxx from the two files in scripts
• decide on what to output if an imported and exported ast-json does not match the original. Expected vs Obtained, diff, both?
• make sure that testscript removes all created files, ALSO when the tests fails. (probably best to move everything in a temporary directory...
• check failing pipeline tests (hmm. this one might be a bigger one, but previous to some tiny commits all tests beside the os_x tests were passing, so I am hoping those that turned up just recently are simple in their nature. fingers crossed)

[djudjuu]

bug3: address(this) has a wrong "referencedDeclaration" [SOLVED]

steps to reproduce

• uncomment those lines and run the test
• expectedResult: the json of [functioncall][arguments][identifier][referencedDeclaration]: 66 (or some other low number
• obtained Result will be 144, (or somehting equally high)

suspected source

"referencedDeclaration" is produced from this:

IdentifierAnnotation& Identifier::annotation() const
{
	if (!m_annotation)
		m_annotation = new IdentifierAnnotation();
	return dynamic_cast<IdentifierAnnotation&>(*m_annotation);
}

so when there is no annotation, it will just produce a new one...

ideas about fixing it

Another thing i think i understood is that the annotations are set in the analyze()-step by the NameAndTypeResolver, which uses m_sourceOrder which in turn is filled in resolveImports() (called first thing in CompilerStack::analyze())

resolveImports relies on m_sources, which is not filled (instead I created m_sourceJsons), but it also refers to annotation() in its toposort()-function. Is it creating those in that step?

So possible avenues for solving it could be to mdofify m_sourceJsons so that it can be used to fill m_sourceOrder??

===========

SOLUTION:

magic variables such as this, block.x are producing some Pseudo-ASTNodes, some of those apparently during parsing. So from now on those will all have nodeID 0

[djudjuu]

current list of code-lines that trigger bugs: [SOLVED, see above]

throw.sol

    require(x != 7, "You must NOT ask for seven");
    assert(x != 77); 
    if (x == 777) { revert(); }

leads to wrong referenced declarations in nodeType Identifier

import.sol

import * as base from "./basicContracts.sol";
contract ABCD is base.A, base.B, base.C, base.D {}

-> first and last element are switched in AST-Node baseContracts
(and more)

address_payable.sol

address(this) (see bug description above)

[chriseth]

 configure ASTImporter to use the evm-version specified in the JSON

The evm version and the ast should be unrelated. The evm version is a setting for the compiler that can be mixed with any source code and the ast is the representation of the source code only, and not of the compiler switches and settings.

[chriseth]

As far as the testing is concerned: Why do you need to store the .json as a file? Wouldn't a usual test just take the solidity source code, export it to json-ast, and compile. Then clear all internal state, re-import from the ast and check that the result is (almost) the same?

[djudjuu]

Wouldn't a usual test just take the solidity source code, export it to json-ast, and compile. Then clear all internal state, re-import from the ast and check that the result is (almost) the same?

Yeah, that would be a better way. The testfile grew as I adapted the ASTJSONTest and I stopped refactoring it once it did the job. I'll put that on the todo-list.

edit: Done

[chriseth]

Ah, this is tricky. Now I think this might have negative impacts later on, because two AST nodes with the same id are considered equal.

[djudjuu]

note: we resolved to have the IDDispenser register all given IDs. still a TODO so i'll leave this around

[chriseth]

Strong objection against this.
We should create a temporary directory at the very beginning and remove that.

[Marenz]

That's what we're doing: https://github.com/ethereum/solidity/pull/7153/files#diff-a8a5760144e03e410389fb0afd20e8c8R440
This is only doing cleanup within that temporary directory for the next test.

[Marenz]

but we could create another tmp directory so we don't use rm in such an intransparent and potentially dangerous way.

[chriseth]

yes, maybe that would be cleaner

[chriseth]

Is splitting the sources and creating actual files really the only option? We cannot use standard-json, for example?

[Marenz]

hmm I could try using the standard-json interface yes

[chriseth]

Keep the current way but do it in a fresh temp dir per run and just delete that, then it's also fine I think.

[Marenz]

I just noticed a complication @chriseth Exporting the version means the tests have a hard-coded version in the json files, which makes testing different evm versions difficult

[Marenz]

I could use something like %EVMVERSION% in the testfiles and search&replace that before testing

[djudjuu]

Whooohoooo!

…

On Tue, Jan 14, 2020, 17:56 chriseth ***@***.***> wrote: Merged #7153 <#7153> into develop. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#7153?email_source=notifications&email_token=ACLMYXHZPYTC4BSZFQZYCT3Q5XVFRA5CNFSM4IH3FITKYY3PNVWWK3TUL52HS4DFWZEXG43VMVCXMZLOORHG65DJMZUWGYLUNFXW5KTDN5WW2ZLOORPWSZGOV62M3NI#event-2947861941>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACLMYXC6AGDNBD3Z4SMPMZ3Q5XVFRANCNFSM4IH3FITA> .