New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support "standardised" JSON compiler input/output #1639

Merged
merged 35 commits into from Apr 20, 2017

Conversation

Projects
None yet
4 participants
@axic
Member

axic commented Feb 2, 2017

Implements #1387. Replaces #712.

Fixes #579, #863, #1645, #1809.

Todo:

  • clean up includes in StandardCompiler.cpp
  • proper error reporting for compiler errors
  • catch exceptions on individual output items (similar to how jsonCompiler works)
  • support AST output
  • support Why3 output
  • support gasEstimates output (depends on #2114)
  • support methodIdentifiers output
  • support linkReferences output
  • control outputs by input selection (likely postponed to a subsequent PR)
  • support URLs in input sources (likely postponed to a subsequent PR)
  • verify input sources against supplied hash (likely postponed to a subsequent PR)
  • support metadata.useLiteralContent
  • add tests

@axic axic changed the title from WIP: Support "standardised" JSON compiler input/output to [WIP] Support "standardised" JSON compiler input/output Feb 6, 2017

@axic

This comment has been minimized.

Show comment
Hide comment
@axic

axic Mar 16, 2017

Member

Working on this next.

Member

axic commented Mar 16, 2017

Working on this next.

@axic axic self-assigned this Mar 16, 2017

@axic

This comment has been minimized.

Show comment
Hide comment
@axic

axic Mar 16, 2017

Member

Added (locally) the JS interface: compileStandard(<string>, <readCallback>) -> <string>.

Any better naming?

Member

axic commented Mar 16, 2017

Added (locally) the JS interface: compileStandard(<string>, <readCallback>) -> <string>.

Any better naming?

@axic

This comment has been minimized.

Show comment
Hide comment
@axic

axic Mar 17, 2017

Member

To keep this PR simple, it will not change the CLI to use StandardCompiler internally. Should be handled in a following PR.

Member

axic commented Mar 17, 2017

To keep this PR simple, it will not change the CLI to use StandardCompiler internally. Should be handled in a following PR.

@chriseth

Did not yet review all of it.

@axic

This comment has been minimized.

Show comment
Hide comment
@axic

axic Mar 29, 2017

Member

Added error reporting of JSON parsing using jsoncpp::getFormattedErrorMessages():

Standard Input JSON: {xxx "language": "Solidity", "sources": { "mortal": { "keccak256": "0x234...", "content": "contract owned { address owner; }x contract mortal is owned { function kill() { if (msg.sender == owner) selfdestruct(owner); } }" } }, "settings": { "remappings": [ ":g/dir" ], "optimizer": { "enabled": true, "runs": 500 }, "metadata": { "useLiteralContent": true }, "libraries": { "myFile.sol": { "MyLib": "0x123123" } }, "outputSelection": { "*": { "*": [ "metadata", "evm.bytecode" ], "": [ "ast", "why3" ] } } } }

Standard Output JSON: {"errors":[{"component":"general","message":"* Line 1, Column 2\n  Missing '}' or object member name\n","severity":"error","type":"JSONError"}]}

It looks a bit weird since it uses asterisk for the list, and maybe we could consider using the sourceLocation parameter for location within the JSON if the type is JSONError.

Member

axic commented Mar 29, 2017

Added error reporting of JSON parsing using jsoncpp::getFormattedErrorMessages():

Standard Input JSON: {xxx "language": "Solidity", "sources": { "mortal": { "keccak256": "0x234...", "content": "contract owned { address owner; }x contract mortal is owned { function kill() { if (msg.sender == owner) selfdestruct(owner); } }" } }, "settings": { "remappings": [ ":g/dir" ], "optimizer": { "enabled": true, "runs": 500 }, "metadata": { "useLiteralContent": true }, "libraries": { "myFile.sol": { "MyLib": "0x123123" } }, "outputSelection": { "*": { "*": [ "metadata", "evm.bytecode" ], "": [ "ast", "why3" ] } } } }

Standard Output JSON: {"errors":[{"component":"general","message":"* Line 1, Column 2\n  Missing '}' or object member name\n","severity":"error","type":"JSONError"}]}

It looks a bit weird since it uses asterisk for the list, and maybe we could consider using the sourceLocation parameter for location within the JSON if the type is JSONError.

@chriseth

Still not finished reviewing everything.

Show outdated Hide outdated libsolidity/interface/StandardCompiler.h
Show outdated Hide outdated libsolidity/interface/StandardCompiler.h
Show outdated Hide outdated libsolidity/interface/StandardCompiler.h
Show outdated Hide outdated libsolidity/interface/StandardCompiler.h
Show outdated Hide outdated libsolidity/interface/StandardCompiler.cpp
Show outdated Hide outdated libsolidity/interface/StandardCompiler.cpp
Show outdated Hide outdated libsolidity/interface/StandardCompiler.cpp
Show outdated Hide outdated libsolidity/interface/StandardCompiler.cpp
for (auto const& ref: linkReferences)
{
string const& fullname = ref.second;
size_t colon = fullname.find(':');

This comment has been minimized.

@chriseth

chriseth Apr 6, 2017

Contributor

Please assert that colon != string::npos

@chriseth

chriseth Apr 6, 2017

Contributor

Please assert that colon != string::npos

This comment has been minimized.

@axic

axic Apr 11, 2017

Member

Added.

@axic

axic Apr 11, 2017

Member

Added.

Show outdated Hide outdated libsolidity/interface/StandardCompiler.cpp
@chriseth

I'm through!

Show outdated Hide outdated solc/CommandLineInterface.cpp
Show outdated Hide outdated solc/CommandLineInterface.cpp
Show outdated Hide outdated solc/CommandLineInterface.cpp
{
readCallback = [=](string const& _path)
{
char* contents_c = nullptr;

This comment has been minimized.

@chriseth

chriseth Apr 7, 2017

Contributor

This code should be duplicated above. Can you reuse it?

@chriseth

chriseth Apr 7, 2017

Contributor

This code should be duplicated above. Can you reuse it?

This comment has been minimized.

@axic

axic Apr 11, 2017

Member

Done.

@axic

axic Apr 11, 2017

Member

Done.

@axic

This comment has been minimized.

Show comment
Hide comment
@axic

axic Apr 7, 2017

Member

@chriseth the CLI part was only added for aiding development. We need to figure out what it should do.

Member

axic commented Apr 7, 2017

@chriseth the CLI part was only added for aiding development. We need to figure out what it should do.

Show outdated Hide outdated libsolidity/interface/StandardCompiler.cpp
Show outdated Hide outdated libsolidity/interface/StandardCompiler.cpp
Show outdated Hide outdated libsolidity/interface/StandardCompiler.cpp
Show outdated Hide outdated libsolidity/interface/StandardCompiler.cpp
Show outdated Hide outdated libsolidity/interface/StandardCompiler.cpp
Show outdated Hide outdated libsolidity/interface/StandardCompiler.cpp
Show outdated Hide outdated libsolidity/interface/StandardCompiler.cpp
Show outdated Hide outdated solc/CommandLineInterface.cpp
Show outdated Hide outdated solc/CommandLineInterface.cpp
@@ -534,6 +537,10 @@ Allowed options)",
)
(g_argGas.c_str(), "Print an estimate of the maximal gas usage for each function.")
(
g_argStandardJSON.c_str(),
"Switch to Standard JSON input / output mode, ignoring all options."

This comment has been minimized.

@chriseth

chriseth Apr 11, 2017

Contributor

In the future, I think we should also allow to read the json from a file and not just from stdin. Can you add an explanation that we expect the input to come from stdin and write the output to stdout?

@chriseth

chriseth Apr 11, 2017

Contributor

In the future, I think we should also allow to read the json from a file and not just from stdin. Can you add an explanation that we expect the input to come from stdin and write the output to stdout?

This comment has been minimized.

@axic

axic Apr 19, 2017

Member

Added.

@axic

axic Apr 19, 2017

Member

Added.

}
catch(...)
{
return "{\"errors\":\"[{\"type\":\"JSONError\",\"component\":\"general\",\"severity\":\"error\",\"message\":\"Error parsing input JSON.\"}]}";

This comment has been minimized.

@pirapira

pirapira Apr 19, 2017

Member

Can this be formatError easily, or is there a reason not to do so?

@pirapira

pirapira Apr 19, 2017

Member

Can this be formatError easily, or is there a reason not to do so?

This comment has been minimized.

@axic

axic Apr 19, 2017

Member

formatError() + jsonCompactPrint() would need to be enclosed in another try/catch in that case

@axic

axic Apr 19, 2017

Member

formatError() + jsonCompactPrint() would need to be enclosed in another try/catch in that case

This comment has been minimized.

@pirapira

pirapira Apr 19, 2017

Member

I see. That's a bit screenful.

@pirapira

pirapira Apr 19, 2017

Member

I see. That's a bit screenful.

Show outdated Hide outdated test/libsolidity/StandardCompiler.cpp
Show outdated Hide outdated test/libsolidity/StandardCompiler.cpp
Show outdated Hide outdated test/libsolidity/StandardCompiler.cpp
BOOST_CHECK(containsError(result, "JSONError", "Input is not a JSON object."));
BOOST_CHECK(!containsError(result, "JSONError", "* Line 1, Column 1\n Syntax error: value, object or array expected.\n"));
result = compile("{}");
BOOST_CHECK(!containsError(result, "JSONError", "* Line 1, Column 1\n Syntax error: value, object or array expected.\n"));

This comment has been minimized.

@chriseth

chriseth Apr 19, 2017

Contributor

Can you add another, positive check here?

@chriseth

chriseth Apr 19, 2017

Contributor

Can you add another, positive check here?

This comment has been minimized.

@axic

axic Apr 19, 2017

Member

It still contains errors covered by the next 3 tests.

@axic

axic Apr 19, 2017

Member

It still contains errors covered by the next 3 tests.

This comment has been minimized.

@chriseth

chriseth Apr 20, 2017

Contributor

Sure, but don't we want to check that it at least contains some error?

@chriseth

chriseth Apr 20, 2017

Contributor

Sure, but don't we want to check that it at least contains some error?

This comment has been minimized.

@axic

axic Apr 20, 2017

Member

How about BOOST_CHECK(!containsAtMostWarnings(result)); then?

@axic

axic Apr 20, 2017

Member

How about BOOST_CHECK(!containsAtMostWarnings(result)); then?

This comment has been minimized.

@axic

axic Apr 20, 2017

Member

Added.

@axic

axic Apr 20, 2017

Member

Added.

Show outdated Hide outdated test/libsolidity/StandardCompiler.cpp
Show outdated Hide outdated test/libsolidity/StandardCompiler.cpp
" mstore(0x40, 0x60)\n tag_1:\n invalid\n}\n");
BOOST_CHECK(contract["evm"]["gasEstimates"].isObject());
BOOST_CHECK(dev::jsonCompactPrint(contract["evm"]["gasEstimates"]) ==
"{\"creation\":{\"codeDepositCost\":\"10200\",\"executionCost\":\"62\",\"totalCost\":\"10262\"}}");

This comment has been minimized.

@chriseth

chriseth Apr 19, 2017

Contributor

Can you check that the numbers are numbers and that's it?

@chriseth

chriseth Apr 19, 2017

Contributor

Can you check that the numbers are numbers and that's it?

This comment has been minimized.

@axic

axic Apr 19, 2017

Member

It is more code to do that 😉

@axic

axic Apr 19, 2017

Member

It is more code to do that 😉

This comment has been minimized.

@chriseth

chriseth Apr 20, 2017

Contributor
BOOST_CHECK(std::regex_match(
    dev::jsonCompactPrint(contract["evm"]["gasEstimates"]),
    std::regex("{\"creation\":{\"codeDepositCost\":\"[0-9]+\",\"executionCost\":\"[0-9]+\",\"totalCost\":\"[0-9]+\"}}")
));
@chriseth

chriseth Apr 20, 2017

Contributor
BOOST_CHECK(std::regex_match(
    dev::jsonCompactPrint(contract["evm"]["gasEstimates"]),
    std::regex("{\"creation\":{\"codeDepositCost\":\"[0-9]+\",\"executionCost\":\"[0-9]+\",\"totalCost\":\"[0-9]+\"}}")
));

This comment has been minimized.

@axic

axic Apr 20, 2017

Member

unknown location:0: fatal error: in "StandardCompiler/basic_compilation": std::__1::regex_error: One of *?+{ was not preceded by a valid regular expression.

@axic

axic Apr 20, 2017

Member

unknown location:0: fatal error: in "StandardCompiler/basic_compilation": std::__1::regex_error: One of *?+{ was not preceded by a valid regular expression.

This comment has been minimized.

@axic

axic Apr 20, 2017

Member

{ and } must be escaped

@axic

axic Apr 20, 2017

Member

{ and } must be escaped

This comment has been minimized.

@axic

axic Apr 20, 2017

Member

Done.

@axic

axic Apr 20, 2017

Member

Done.

@axic axic changed the title from [WIP] Support "standardised" JSON compiler input/output to Support "standardised" JSON compiler input/output Apr 19, 2017

@axic

This comment has been minimized.

Show comment
Hide comment
@axic

axic Apr 20, 2017

Member

@chriseth the regex fails on travis :(

Member

axic commented Apr 20, 2017

@chriseth the regex fails on travis :(

@axic

This comment has been minimized.

Show comment
Hide comment
@axic

axic Apr 20, 2017

Member

Depends on #2143.

Member

axic commented Apr 20, 2017

Depends on #2143.

@axic axic merged commit 2ccbc08 into develop Apr 20, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@axic axic removed the in progress label Apr 20, 2017

@axic axic deleted the json-interface-api branch Apr 20, 2017

@pipermerriam

This comment has been minimized.

Show comment
Hide comment
@pipermerriam

pipermerriam Apr 20, 2017

Member

Woooohooo!!!! 🍰 !

Member

pipermerriam commented Apr 20, 2017

Woooohooo!!!! 🍰 !

@cag cag referenced this pull request May 1, 2017

Closed

Testing issues #287

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment