From 60ac4d3a2c3960edec7db7b9e1328c2597c406e9 Mon Sep 17 00:00:00 2001 From: Lefteris Karapetsas Date: Mon, 1 Dec 2014 17:03:04 +0100 Subject: [PATCH 01/27] Exporting Natspec documentation to a JSON interface - Adding a getDocumentation() function to solidity compiler stack so that we can obtain the natspec interface for a contract - Adding libjsoncpp as a dependency of libsolidity. This is done in a dirty way, using libjsonrpc-cpp s an intermediate dependency for the moment. Will fix soon. - Start of a test file for Natspec exporting to JSON --- libsolidity/AST.h | 2 +- libsolidity/CMakeLists.txt | 4 ++ libsolidity/CompilerStack.cpp | 26 ++++++++++++ libsolidity/CompilerStack.h | 4 ++ test/solidityNatspecJSON.cpp | 77 +++++++++++++++++++++++++++++++++++ 5 files changed, 112 insertions(+), 1 deletion(-) create mode 100644 test/solidityNatspecJSON.cpp diff --git a/libsolidity/AST.h b/libsolidity/AST.h index 81a12ad1a..4ed8489fc 100644 --- a/libsolidity/AST.h +++ b/libsolidity/AST.h @@ -199,7 +199,7 @@ public: Block& getBody() { return *m_body; } /// @return A shared pointer of an ASTString. /// Can contain a nullptr in which case indicates absence of documentation - ASTPointer const& getDocumentation() { return m_documentation; } + ASTPointer const& getDocumentation() const { return m_documentation; } void addLocalVariable(VariableDeclaration const& _localVariable) { m_localVariables.push_back(&_localVariable); } std::vector const& getLocalVariables() const { return m_localVariables; } diff --git a/libsolidity/CMakeLists.txt b/libsolidity/CMakeLists.txt index ea2ef4b74..b5147ced3 100644 --- a/libsolidity/CMakeLists.txt +++ b/libsolidity/CMakeLists.txt @@ -16,6 +16,10 @@ endif() include_directories(..) target_link_libraries(${EXECUTABLE} evmcore devcore) +# TODO: Temporary until PR 532 https://github.com/ethereum/cpp-ethereum/pull/532 +# gets accepted. Then we can simply add jsoncpp as a dependency and not the +# whole of JSONRPC as we are doing right here +target_link_libraries(${EXECUTABLE} ${JSONRPC_LS}) install( TARGETS ${EXECUTABLE} ARCHIVE DESTINATION lib LIBRARY DESTINATION lib ) install( FILES ${HEADERS} DESTINATION include/${EXECUTABLE} ) diff --git a/libsolidity/CompilerStack.cpp b/libsolidity/CompilerStack.cpp index 6535e00d4..45d3e0b8a 100644 --- a/libsolidity/CompilerStack.cpp +++ b/libsolidity/CompilerStack.cpp @@ -28,6 +28,8 @@ #include #include +#include + using namespace std; namespace dev @@ -125,6 +127,30 @@ string const& CompilerStack::getInterface() return m_interface; } +string const& CompilerStack::getDocumentation() +{ + + Json::StyledWriter writer; + if (!m_parseSuccessful) + BOOST_THROW_EXCEPTION(CompilerError() << errinfo_comment("Parsing was not successful.")); + if (m_documentation.empty()) + { + Json::Value doc; + Json::Value methods; + + vector exportedFunctions = m_contractASTNode->getInterfaceFunctions(); + for (FunctionDefinition const* f: exportedFunctions) + { + Json::Value user; + user["user"] = Json::Value(*f->getDocumentation()); + methods[f->getName()] = user; + } + doc["methods"] = methods; + m_documentation = writer.write(doc); + } + return m_documentation; +} + bytes CompilerStack::staticCompile(std::string const& _sourceCode, bool _optimize) { CompilerStack stack; diff --git a/libsolidity/CompilerStack.h b/libsolidity/CompilerStack.h index 6cae8660f..74784c5ef 100644 --- a/libsolidity/CompilerStack.h +++ b/libsolidity/CompilerStack.h @@ -62,6 +62,9 @@ public: /// Returns a string representing the contract interface in JSON. /// Prerequisite: Successful call to parse or compile. std::string const& getInterface(); + /// Returns a string representing the contract documentation in JSON. + /// Prerequisite: Successful call to parse or compile. + std::string const& getDocumentation(); /// Returns the previously used scanner, useful for counting lines during error reporting. Scanner const& getScanner() const { return *m_scanner; } @@ -77,6 +80,7 @@ private: std::shared_ptr m_contractASTNode; bool m_parseSuccessful; std::string m_interface; + std::string m_documentation; std::shared_ptr m_compiler; bytes m_bytecode; }; diff --git a/test/solidityNatspecJSON.cpp b/test/solidityNatspecJSON.cpp new file mode 100644 index 000000000..5918eec99 --- /dev/null +++ b/test/solidityNatspecJSON.cpp @@ -0,0 +1,77 @@ +/* + This file is part of cpp-ethereum. + + cpp-ethereum is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + cpp-ethereum is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with cpp-ethereum. If not, see . + */ +/** + * @author Lefteris Karapetsas + * @date 2014 + * Unit tests for the solidity compiler JSON Interface output. + */ + +#include +#include +#include + +namespace dev +{ +namespace solidity +{ +namespace test +{ + +class DocumentationChecker +{ +public: + void checkNatspec(std::string const& _code, std::string const& _expectedDocumentationString) + { + m_compilerStack.parse(_code); + auto generatedDocumentationString = m_compilerStack.getDocumentation(); + Json::Value generatedDocumentation; + m_reader.parse(generatedDocumentationString, generatedDocumentation); + Json::Value expectedDocumentation; + m_reader.parse(_expectedDocumentationString, expectedDocumentation); + BOOST_CHECK_MESSAGE(expectedDocumentation == generatedDocumentation, + "Expected " << _expectedDocumentationString << + "\n but got:\n" << generatedDocumentationString); + } + +private: + CompilerStack m_compilerStack; + Json::Reader m_reader; +}; + +BOOST_FIXTURE_TEST_SUITE(SolidityNatspecJSON, DocumentationChecker) + +BOOST_AUTO_TEST_CASE(basic_test) +{ + char const* sourceCode = "contract test {\n" + " /// Multiplies `a` by 7\n" + " function mul(uint a) returns(uint d) { return a * 7; }\n" + "}\n"; + + char const* natspec = "{" + "\"methods\":{" + " \"mul\":{ \"user\": \" Multiplies `a` by 7\"}" + "}}"; + + checkNatspec(sourceCode, natspec); +} + + +BOOST_AUTO_TEST_SUITE_END() + +} +} +} From eed32824c3eb5ebd271d7cff585655b48ab5f697 Mon Sep 17 00:00:00 2001 From: Lefteris Karapetsas Date: Mon, 1 Dec 2014 18:01:42 +0100 Subject: [PATCH 02/27] Using jsoncpp for exporting ABI interface from solidity - Also changing the interface JSON test to have a shorter name plus to provide meaningful error message in case of failure --- libsolidity/CompilerStack.cpp | 48 ++++++++++++++---------------- test/solidityJSONInterfaceTest.cpp | 26 ++++++++-------- 2 files changed, 37 insertions(+), 37 deletions(-) diff --git a/libsolidity/CompilerStack.cpp b/libsolidity/CompilerStack.cpp index 45d3e0b8a..e25438f7c 100644 --- a/libsolidity/CompilerStack.cpp +++ b/libsolidity/CompilerStack.cpp @@ -85,54 +85,52 @@ void CompilerStack::streamAssembly(ostream& _outStream) string const& CompilerStack::getInterface() { + Json::StyledWriter writer; if (!m_parseSuccessful) BOOST_THROW_EXCEPTION(CompilerError() << errinfo_comment("Parsing was not successful.")); + if (m_interface.empty()) { - stringstream interface; - interface << '['; + Json::Value methods(Json::arrayValue); + vector exportedFunctions = m_contractASTNode->getInterfaceFunctions(); - unsigned functionsCount = exportedFunctions.size(); for (FunctionDefinition const* f: exportedFunctions) { - auto streamVariables = [&](vector> const& _vars) + Json::Value method; + Json::Value inputs(Json::arrayValue); + Json::Value outputs(Json::arrayValue); + + auto streamVariables = [&](vector> const& _vars, + Json::Value &json) { - unsigned varCount = _vars.size(); for (ASTPointer const& var: _vars) { - interface << "{" - << "\"name\":" << escaped(var->getName(), false) << "," - << "\"type\":" << escaped(var->getType()->toString(), false) - << "}"; - if (--varCount > 0) - interface << ","; + Json::Value input; + input["name"] = var->getName(); + input["type"] = var->getType()->toString(); + json.append(input); } }; - interface << '{' - << "\"name\":" << escaped(f->getName(), false) << "," - << "\"inputs\":["; - streamVariables(f->getParameters()); - interface << "]," - << "\"outputs\":["; - streamVariables(f->getReturnParameters()); - interface << "]" - << "}"; - if (--functionsCount > 0) - interface << ","; + method["name"] = f->getName(); + streamVariables(f->getParameters(), inputs); + method["inputs"] = inputs; + streamVariables(f->getReturnParameters(), outputs); + method["outputs"] = outputs; + + methods.append(method); } - interface << ']'; - m_interface = interface.str(); + m_interface = writer.write(methods); } return m_interface; } string const& CompilerStack::getDocumentation() { - Json::StyledWriter writer; if (!m_parseSuccessful) BOOST_THROW_EXCEPTION(CompilerError() << errinfo_comment("Parsing was not successful.")); + if (m_documentation.empty()) { Json::Value doc; diff --git a/test/solidityJSONInterfaceTest.cpp b/test/solidityJSONInterfaceTest.cpp index 1a443087f..b91880130 100644 --- a/test/solidityJSONInterfaceTest.cpp +++ b/test/solidityJSONInterfaceTest.cpp @@ -34,7 +34,7 @@ namespace test class InterfaceChecker { public: - bool checkInterface(std::string const& _code, std::string const& _expectedInterfaceString) + void checkInterface(std::string const& _code, std::string const& _expectedInterfaceString) { m_compilerStack.parse(_code); std::string generatedInterfaceString = m_compilerStack.getInterface(); @@ -42,15 +42,17 @@ public: m_reader.parse(generatedInterfaceString, generatedInterface); Json::Value expectedInterface; m_reader.parse(_expectedInterfaceString, expectedInterface); - return expectedInterface == generatedInterface; + BOOST_CHECK_MESSAGE(expectedInterface == generatedInterface, + "Expected " << _expectedInterfaceString << + "\n but got:\n" << generatedInterfaceString); } - + private: CompilerStack m_compilerStack; Json::Reader m_reader; }; -BOOST_FIXTURE_TEST_SUITE(SolidityCompilerJSONInterfaceOutput, InterfaceChecker) +BOOST_FIXTURE_TEST_SUITE(solidityABIJSON, InterfaceChecker) BOOST_AUTO_TEST_CASE(basic_test) { @@ -76,7 +78,7 @@ BOOST_AUTO_TEST_CASE(basic_test) } ])"; - BOOST_CHECK(checkInterface(sourceCode, interface)); + checkInterface(sourceCode, interface); } BOOST_AUTO_TEST_CASE(empty_contract) @@ -86,7 +88,7 @@ BOOST_AUTO_TEST_CASE(empty_contract) char const* interface = "[]"; - BOOST_CHECK(checkInterface(sourceCode, interface)); + checkInterface(sourceCode, interface); } BOOST_AUTO_TEST_CASE(multiple_methods) @@ -95,7 +97,7 @@ BOOST_AUTO_TEST_CASE(multiple_methods) " function f(uint a) returns(uint d) { return a * 7; }\n" " function g(uint b) returns(uint e) { return b * 8; }\n" "}\n"; - + char const* interface = R"([ { "name": "f", @@ -129,7 +131,7 @@ BOOST_AUTO_TEST_CASE(multiple_methods) } ])"; - BOOST_CHECK(checkInterface(sourceCode, interface)); + checkInterface(sourceCode, interface); } BOOST_AUTO_TEST_CASE(multiple_params) @@ -160,7 +162,7 @@ BOOST_AUTO_TEST_CASE(multiple_params) } ])"; - BOOST_CHECK(checkInterface(sourceCode, interface)); + checkInterface(sourceCode, interface); } BOOST_AUTO_TEST_CASE(multiple_methods_order) @@ -170,7 +172,7 @@ BOOST_AUTO_TEST_CASE(multiple_methods_order) " function f(uint a) returns(uint d) { return a * 7; }\n" " function c(uint b) returns(uint e) { return b * 8; }\n" "}\n"; - + char const* interface = R"([ { "name": "c", @@ -203,8 +205,8 @@ BOOST_AUTO_TEST_CASE(multiple_methods_order) ] } ])"; - - BOOST_CHECK(checkInterface(sourceCode, interface)); + + checkInterface(sourceCode, interface); } BOOST_AUTO_TEST_SUITE_END() From 88b1bb25404bba285f903668ae0d3c44642d9a92 Mon Sep 17 00:00:00 2001 From: Lefteris Karapetsas Date: Tue, 2 Dec 2014 10:41:18 +0100 Subject: [PATCH 03/27] More Natspec JSON export tests and better error reporting --- libsolidity/CompilerStack.cpp | 2 +- test/solidityJSONInterfaceTest.cpp | 17 ++++++- test/solidityNatspecJSON.cpp | 76 +++++++++++++++++++++++++++++- 3 files changed, 91 insertions(+), 4 deletions(-) diff --git a/libsolidity/CompilerStack.cpp b/libsolidity/CompilerStack.cpp index e25438f7c..77a019b5f 100644 --- a/libsolidity/CompilerStack.cpp +++ b/libsolidity/CompilerStack.cpp @@ -134,7 +134,7 @@ string const& CompilerStack::getDocumentation() if (m_documentation.empty()) { Json::Value doc; - Json::Value methods; + Json::Value methods(Json::objectValue); vector exportedFunctions = m_contractASTNode->getInterfaceFunctions(); for (FunctionDefinition const* f: exportedFunctions) diff --git a/test/solidityJSONInterfaceTest.cpp b/test/solidityJSONInterfaceTest.cpp index b91880130..f46a3ad36 100644 --- a/test/solidityJSONInterfaceTest.cpp +++ b/test/solidityJSONInterfaceTest.cpp @@ -23,6 +23,7 @@ #include #include #include +#include namespace dev { @@ -36,7 +37,19 @@ class InterfaceChecker public: void checkInterface(std::string const& _code, std::string const& _expectedInterfaceString) { - m_compilerStack.parse(_code); + try + { + m_compilerStack.parse(_code); + } + catch (const std::exception& e) + { + std::string const* extra = boost::get_error_info(e); + std::string msg = std::string("Parsing contract failed with: ") + + e.what() + std::string("\n"); + if (extra) + msg += *extra; + BOOST_FAIL(msg); + } std::string generatedInterfaceString = m_compilerStack.getInterface(); Json::Value generatedInterface; m_reader.parse(generatedInterfaceString, generatedInterface); @@ -52,7 +65,7 @@ private: Json::Reader m_reader; }; -BOOST_FIXTURE_TEST_SUITE(solidityABIJSON, InterfaceChecker) +BOOST_FIXTURE_TEST_SUITE(SolidityABIJSON, InterfaceChecker) BOOST_AUTO_TEST_CASE(basic_test) { diff --git a/test/solidityNatspecJSON.cpp b/test/solidityNatspecJSON.cpp index 5918eec99..5a4649d2e 100644 --- a/test/solidityNatspecJSON.cpp +++ b/test/solidityNatspecJSON.cpp @@ -23,6 +23,7 @@ #include #include #include +#include namespace dev { @@ -36,7 +37,19 @@ class DocumentationChecker public: void checkNatspec(std::string const& _code, std::string const& _expectedDocumentationString) { - m_compilerStack.parse(_code); + try + { + m_compilerStack.parse(_code); + } + catch (const std::exception& e) + { + std::string const* extra = boost::get_error_info(e); + std::string msg = std::string("Parsing contract failed with: ") + + e.what() + std::string("\n"); + if (extra) + msg += *extra; + BOOST_FAIL(msg); + } auto generatedDocumentationString = m_compilerStack.getDocumentation(); Json::Value generatedDocumentation; m_reader.parse(generatedDocumentationString, generatedDocumentation); @@ -69,6 +82,67 @@ BOOST_AUTO_TEST_CASE(basic_test) checkNatspec(sourceCode, natspec); } +BOOST_AUTO_TEST_CASE(multiline_comment) +{ + char const* sourceCode = "contract test {\n" + " /// Multiplies `a` by 7\n" + " /// and then adds `b`\n" + " function mul_and_add(uint a, uint256 b) returns(uint256 d)\n" + " {\n" + " return (a * 7) + b;\n" + " }\n" + "}\n"; + + char const* natspec = "{" + "\"methods\":{" + " \"mul_and_add\":{ \"user\": \" Multiplies `a` by 7\n and then adds `b`\"}" + "}}"; + + checkNatspec(sourceCode, natspec); +} + +BOOST_AUTO_TEST_CASE(multiple_functions) +{ + char const* sourceCode = "contract test {\n" + " /// Multiplies `a` by 7\n" + " /// and then adds `b`\n" + " function mul_and_add(uint a, uint256 b) returns(uint256 d)\n" + " {\n" + " return (a * 7) + b;\n" + " }\n" + "\n" + " /// Divides `input` by `div`\n" + " function divide(uint input, uint div) returns(uint d)\n" + " {\n" + " return input / div;\n" + " }\n" + " /// Subtracts 3 from `input`\n" + " function sub(int input) returns(int d)\n" + " {\n" + " return input - 3;\n" + " }\n" + "}\n"; + + char const* natspec = "{" + "\"methods\":{" + " \"mul_and_add\":{ \"user\": \" Multiplies `a` by 7\n and then adds `b`\"}," + " \"divide\":{ \"user\": \" Divides `input` by `div`\"}," + " \"sub\":{ \"user\": \" Subtracts 3 from `input`\"}" + "}}"; + + checkNatspec(sourceCode, natspec); +} + +BOOST_AUTO_TEST_CASE(empty_contract) +{ + char const* sourceCode = "contract test {\n" + "}\n"; + + char const* natspec = "{\"methods\":{} }"; + + checkNatspec(sourceCode, natspec); +} + BOOST_AUTO_TEST_SUITE_END() From 93c488ce736f9f26edb556a4e112b188bdf4465e Mon Sep 17 00:00:00 2001 From: Lefteris Karapetsas Date: Tue, 2 Dec 2014 11:03:34 +0100 Subject: [PATCH 04/27] Handle absence of Natspec doc and add option to solc --- libsolidity/CompilerStack.cpp | 8 ++++++-- solc/main.cpp | 1 + 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/libsolidity/CompilerStack.cpp b/libsolidity/CompilerStack.cpp index 77a019b5f..2b2991584 100644 --- a/libsolidity/CompilerStack.cpp +++ b/libsolidity/CompilerStack.cpp @@ -140,8 +140,12 @@ string const& CompilerStack::getDocumentation() for (FunctionDefinition const* f: exportedFunctions) { Json::Value user; - user["user"] = Json::Value(*f->getDocumentation()); - methods[f->getName()] = user; + auto strPtr = f->getDocumentation(); + if (strPtr) + { + user["user"] = Json::Value(*strPtr); + methods[f->getName()] = user; + } } doc["methods"] = methods; m_documentation = writer.write(doc); diff --git a/solc/main.cpp b/solc/main.cpp index a7216e594..6ca130a79 100644 --- a/solc/main.cpp +++ b/solc/main.cpp @@ -136,6 +136,7 @@ int main(int argc, char** argv) cout << eth::disassemble(compiler.getBytecode()) << endl; cout << "Binary: " << toHex(compiler.getBytecode()) << endl; cout << "Interface specification: " << compiler.getInterface() << endl; + cout << "Natspec documentation: " << compiler.getDocumentation() << endl; return 0; } From c89fc7df63fef3e430e17550e28083d71e3db647 Mon Sep 17 00:00:00 2001 From: Lefteris Karapetsas Date: Tue, 2 Dec 2014 12:14:24 +0100 Subject: [PATCH 05/27] Removing unneeded local variable in CompilerStack::getDocumentation() --- libsolidity/CompilerStack.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/libsolidity/CompilerStack.cpp b/libsolidity/CompilerStack.cpp index 2b2991584..48a37b9cf 100644 --- a/libsolidity/CompilerStack.cpp +++ b/libsolidity/CompilerStack.cpp @@ -136,8 +136,7 @@ string const& CompilerStack::getDocumentation() Json::Value doc; Json::Value methods(Json::objectValue); - vector exportedFunctions = m_contractASTNode->getInterfaceFunctions(); - for (FunctionDefinition const* f: exportedFunctions) + for (FunctionDefinition const* f: m_contractASTNode->getInterfaceFunctions()) { Json::Value user; auto strPtr = f->getDocumentation(); From 9ff245ab52bd35cf7fa5ad845df9cdd7bd693f7c Mon Sep 17 00:00:00 2001 From: Lefteris Karapetsas Date: Tue, 2 Dec 2014 17:18:09 +0100 Subject: [PATCH 06/27] Simplifying lambda function in CompilerStack::getInterface() --- libsolidity/CompilerStack.cpp | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/libsolidity/CompilerStack.cpp b/libsolidity/CompilerStack.cpp index 48a37b9cf..e44a10fbe 100644 --- a/libsolidity/CompilerStack.cpp +++ b/libsolidity/CompilerStack.cpp @@ -100,24 +100,22 @@ string const& CompilerStack::getInterface() Json::Value inputs(Json::arrayValue); Json::Value outputs(Json::arrayValue); - auto streamVariables = [&](vector> const& _vars, - Json::Value &json) + auto streamVariables = [](vector> const& _vars) { + Json::Value params(Json::arrayValue); for (ASTPointer const& var: _vars) { Json::Value input; input["name"] = var->getName(); input["type"] = var->getType()->toString(); - json.append(input); + params.append(input); } + return params; }; method["name"] = f->getName(); - streamVariables(f->getParameters(), inputs); - method["inputs"] = inputs; - streamVariables(f->getReturnParameters(), outputs); - method["outputs"] = outputs; - + method["inputs"] = streamVariables(f->getParameters()); + method["outputs"] = streamVariables(f->getReturnParameters()); methods.append(method); } m_interface = writer.write(methods); From 994e891078c0197d6c6b0347d4040f08f6564958 Mon Sep 17 00:00:00 2001 From: Lefteris Karapetsas Date: Wed, 3 Dec 2014 13:50:04 +0100 Subject: [PATCH 07/27] Separate user and dev natspec documentation - plus other small changes according to the spec --- libsolidity/CompilerStack.cpp | 23 +++++++++++++++++----- libsolidity/CompilerStack.h | 10 +++++++--- solc/main.cpp | 2 +- test/solidityNatspecJSON.cpp | 37 +++++++++++++++++++++-------------- 4 files changed, 48 insertions(+), 24 deletions(-) diff --git a/libsolidity/CompilerStack.cpp b/libsolidity/CompilerStack.cpp index e44a10fbe..b12097052 100644 --- a/libsolidity/CompilerStack.cpp +++ b/libsolidity/CompilerStack.cpp @@ -123,13 +123,13 @@ string const& CompilerStack::getInterface() return m_interface; } -string const& CompilerStack::getDocumentation() +string const& CompilerStack::getUserDocumentation() { Json::StyledWriter writer; if (!m_parseSuccessful) BOOST_THROW_EXCEPTION(CompilerError() << errinfo_comment("Parsing was not successful.")); - if (m_documentation.empty()) + if (m_userDocumentation.empty()) { Json::Value doc; Json::Value methods(Json::objectValue); @@ -140,14 +140,27 @@ string const& CompilerStack::getDocumentation() auto strPtr = f->getDocumentation(); if (strPtr) { - user["user"] = Json::Value(*strPtr); + user["notice"] = Json::Value(*strPtr); methods[f->getName()] = user; } } doc["methods"] = methods; - m_documentation = writer.write(doc); + m_userDocumentation = writer.write(doc); } - return m_documentation; + return m_userDocumentation; +} + +string const& CompilerStack::getDevDocumentation() +{ + Json::StyledWriter writer; + if (!m_parseSuccessful) + BOOST_THROW_EXCEPTION(CompilerError() << errinfo_comment("Parsing was not successful.")); + + if (m_devDocumentation.empty()) + { + // TODO + } + return m_devDocumentation; } bytes CompilerStack::staticCompile(std::string const& _sourceCode, bool _optimize) diff --git a/libsolidity/CompilerStack.h b/libsolidity/CompilerStack.h index 74784c5ef..4e0d2251b 100644 --- a/libsolidity/CompilerStack.h +++ b/libsolidity/CompilerStack.h @@ -62,9 +62,12 @@ public: /// Returns a string representing the contract interface in JSON. /// Prerequisite: Successful call to parse or compile. std::string const& getInterface(); - /// Returns a string representing the contract documentation in JSON. + /// Returns a string representing the contract's user documentation in JSON. /// Prerequisite: Successful call to parse or compile. - std::string const& getDocumentation(); + std::string const& getUserDocumentation(); + /// Returns a string representing the contract's developer documentation in JSON. + /// Prerequisite: Successful call to parse or compile. + std::string const& getDevDocumentation(); /// Returns the previously used scanner, useful for counting lines during error reporting. Scanner const& getScanner() const { return *m_scanner; } @@ -80,7 +83,8 @@ private: std::shared_ptr m_contractASTNode; bool m_parseSuccessful; std::string m_interface; - std::string m_documentation; + std::string m_userDocumentation; + std::string m_devDocumentation; std::shared_ptr m_compiler; bytes m_bytecode; }; diff --git a/solc/main.cpp b/solc/main.cpp index 6ca130a79..29239a717 100644 --- a/solc/main.cpp +++ b/solc/main.cpp @@ -136,7 +136,7 @@ int main(int argc, char** argv) cout << eth::disassemble(compiler.getBytecode()) << endl; cout << "Binary: " << toHex(compiler.getBytecode()) << endl; cout << "Interface specification: " << compiler.getInterface() << endl; - cout << "Natspec documentation: " << compiler.getDocumentation() << endl; + cout << "Natspec user documentation: " << compiler.getUserDocumentation() << endl; return 0; } diff --git a/test/solidityNatspecJSON.cpp b/test/solidityNatspecJSON.cpp index 5a4649d2e..f729e4ff8 100644 --- a/test/solidityNatspecJSON.cpp +++ b/test/solidityNatspecJSON.cpp @@ -35,8 +35,11 @@ namespace test class DocumentationChecker { public: - void checkNatspec(std::string const& _code, std::string const& _expectedDocumentationString) + void checkNatspec(std::string const& _code, + std::string const& _expectedDocumentationString, + bool _userDocumentation) { + std::string generatedDocumentationString; try { m_compilerStack.parse(_code); @@ -50,7 +53,11 @@ public: msg += *extra; BOOST_FAIL(msg); } - auto generatedDocumentationString = m_compilerStack.getDocumentation(); + + if (_userDocumentation) + generatedDocumentationString = m_compilerStack.getUserDocumentation(); + else + generatedDocumentationString = m_compilerStack.getDevDocumentation(); Json::Value generatedDocumentation; m_reader.parse(generatedDocumentationString, generatedDocumentation); Json::Value expectedDocumentation; @@ -67,7 +74,7 @@ private: BOOST_FIXTURE_TEST_SUITE(SolidityNatspecJSON, DocumentationChecker) -BOOST_AUTO_TEST_CASE(basic_test) +BOOST_AUTO_TEST_CASE(user_basic_test) { char const* sourceCode = "contract test {\n" " /// Multiplies `a` by 7\n" @@ -76,13 +83,13 @@ BOOST_AUTO_TEST_CASE(basic_test) char const* natspec = "{" "\"methods\":{" - " \"mul\":{ \"user\": \" Multiplies `a` by 7\"}" + " \"mul\":{ \"notice\": \" Multiplies `a` by 7\"}" "}}"; - checkNatspec(sourceCode, natspec); + checkNatspec(sourceCode, natspec, true); } -BOOST_AUTO_TEST_CASE(multiline_comment) +BOOST_AUTO_TEST_CASE(user_multiline_comment) { char const* sourceCode = "contract test {\n" " /// Multiplies `a` by 7\n" @@ -95,13 +102,13 @@ BOOST_AUTO_TEST_CASE(multiline_comment) char const* natspec = "{" "\"methods\":{" - " \"mul_and_add\":{ \"user\": \" Multiplies `a` by 7\n and then adds `b`\"}" + " \"mul_and_add\":{ \"notice\": \" Multiplies `a` by 7\n and then adds `b`\"}" "}}"; - checkNatspec(sourceCode, natspec); + checkNatspec(sourceCode, natspec, true); } -BOOST_AUTO_TEST_CASE(multiple_functions) +BOOST_AUTO_TEST_CASE(user_multiple_functions) { char const* sourceCode = "contract test {\n" " /// Multiplies `a` by 7\n" @@ -125,22 +132,22 @@ BOOST_AUTO_TEST_CASE(multiple_functions) char const* natspec = "{" "\"methods\":{" - " \"mul_and_add\":{ \"user\": \" Multiplies `a` by 7\n and then adds `b`\"}," - " \"divide\":{ \"user\": \" Divides `input` by `div`\"}," - " \"sub\":{ \"user\": \" Subtracts 3 from `input`\"}" + " \"mul_and_add\":{ \"notice\": \" Multiplies `a` by 7\n and then adds `b`\"}," + " \"divide\":{ \"notice\": \" Divides `input` by `div`\"}," + " \"sub\":{ \"notice\": \" Subtracts 3 from `input`\"}" "}}"; - checkNatspec(sourceCode, natspec); + checkNatspec(sourceCode, natspec, true); } -BOOST_AUTO_TEST_CASE(empty_contract) +BOOST_AUTO_TEST_CASE(user_empty_contract) { char const* sourceCode = "contract test {\n" "}\n"; char const* natspec = "{\"methods\":{} }"; - checkNatspec(sourceCode, natspec); + checkNatspec(sourceCode, natspec, true); } From 4613214098850490c0da24cfca5c36e5ce014512 Mon Sep 17 00:00:00 2001 From: Lefteris Karapetsas Date: Wed, 3 Dec 2014 16:40:37 +0100 Subject: [PATCH 08/27] Moving all Interface and Documentation functionality to own class - Creating the Interface Handler class which will take care of the parsing of Natspec comments and of interfacing with and outputing to JSON files. - Will also handle the ABI interface creation --- libsolidity/CompilerStack.cpp | 90 +++++++----------------------- libsolidity/CompilerStack.h | 26 ++++++--- libsolidity/InterfaceHandler.cpp | 88 +++++++++++++++++++++++++++++ libsolidity/InterfaceHandler.h | 74 ++++++++++++++++++++++++ solc/main.cpp | 5 +- test/solidityJSONInterfaceTest.cpp | 2 +- test/solidityNatspecJSON.cpp | 4 +- 7 files changed, 205 insertions(+), 84 deletions(-) create mode 100644 libsolidity/InterfaceHandler.cpp create mode 100644 libsolidity/InterfaceHandler.h diff --git a/libsolidity/CompilerStack.cpp b/libsolidity/CompilerStack.cpp index b12097052..6f80d245c 100644 --- a/libsolidity/CompilerStack.cpp +++ b/libsolidity/CompilerStack.cpp @@ -27,8 +27,7 @@ #include #include #include - -#include +#include using namespace std; @@ -37,6 +36,8 @@ namespace dev namespace solidity { +CompilerStack::CompilerStack():m_interfaceHandler(make_shared()){} + void CompilerStack::setSource(string const& _sourceCode) { reset(); @@ -83,84 +84,33 @@ void CompilerStack::streamAssembly(ostream& _outStream) m_compiler->streamAssembly(_outStream); } -string const& CompilerStack::getInterface() -{ - Json::StyledWriter writer; - if (!m_parseSuccessful) - BOOST_THROW_EXCEPTION(CompilerError() << errinfo_comment("Parsing was not successful.")); - - if (m_interface.empty()) - { - Json::Value methods(Json::arrayValue); - - vector exportedFunctions = m_contractASTNode->getInterfaceFunctions(); - for (FunctionDefinition const* f: exportedFunctions) - { - Json::Value method; - Json::Value inputs(Json::arrayValue); - Json::Value outputs(Json::arrayValue); - - auto streamVariables = [](vector> const& _vars) - { - Json::Value params(Json::arrayValue); - for (ASTPointer const& var: _vars) - { - Json::Value input; - input["name"] = var->getName(); - input["type"] = var->getType()->toString(); - params.append(input); - } - return params; - }; - - method["name"] = f->getName(); - method["inputs"] = streamVariables(f->getParameters()); - method["outputs"] = streamVariables(f->getReturnParameters()); - methods.append(method); - } - m_interface = writer.write(methods); - } - return m_interface; -} - -string const& CompilerStack::getUserDocumentation() +std::string const* CompilerStack::getJsonDocumentation(enum documentation_type _type) { - Json::StyledWriter writer; if (!m_parseSuccessful) BOOST_THROW_EXCEPTION(CompilerError() << errinfo_comment("Parsing was not successful.")); - if (m_userDocumentation.empty()) + auto createOrReturnDoc = [&, this](std::unique_ptr& _doc) { - Json::Value doc; - Json::Value methods(Json::objectValue); - - for (FunctionDefinition const* f: m_contractASTNode->getInterfaceFunctions()) + if(!_doc) { - Json::Value user; - auto strPtr = f->getDocumentation(); - if (strPtr) - { - user["notice"] = Json::Value(*strPtr); - methods[f->getName()] = user; - } + _doc = m_interfaceHandler->getDocumentation(m_contractASTNode, _type); } - doc["methods"] = methods; - m_userDocumentation = writer.write(doc); - } - return m_userDocumentation; -} - -string const& CompilerStack::getDevDocumentation() -{ - Json::StyledWriter writer; - if (!m_parseSuccessful) - BOOST_THROW_EXCEPTION(CompilerError() << errinfo_comment("Parsing was not successful.")); + }; - if (m_devDocumentation.empty()) + switch (_type) { - // TODO + case NATSPEC_USER: + createOrReturnDoc(m_userDocumentation); + return m_userDocumentation.get(); + case NATSPEC_DEV: + createOrReturnDoc(m_devDocumentation); + return m_devDocumentation.get(); + case ABI_INTERFACE: + createOrReturnDoc(m_interface); + return m_interface.get(); } - return m_devDocumentation; + BOOST_THROW_EXCEPTION(CompilerError() << errinfo_comment("Internal error")); + return nullptr; } bytes CompilerStack::staticCompile(std::string const& _sourceCode, bool _optimize) diff --git a/libsolidity/CompilerStack.h b/libsolidity/CompilerStack.h index 4e0d2251b..7dc86e2be 100644 --- a/libsolidity/CompilerStack.h +++ b/libsolidity/CompilerStack.h @@ -35,6 +35,14 @@ class Scanner; class ContractDefinition; class Compiler; class GlobalContext; +class InterfaceHandler; + +enum documentation_type : unsigned short +{ + NATSPEC_USER = 1, + NATSPEC_DEV, + ABI_INTERFACE +}; /** * Easy to use and self-contained Solidity compiler with as few header dependencies as possible. @@ -44,7 +52,7 @@ class GlobalContext; class CompilerStack { public: - CompilerStack() {} + CompilerStack(); void reset() { *this = CompilerStack(); } void setSource(std::string const& _sourceCode); void parse(); @@ -62,12 +70,11 @@ public: /// Returns a string representing the contract interface in JSON. /// Prerequisite: Successful call to parse or compile. std::string const& getInterface(); - /// Returns a string representing the contract's user documentation in JSON. - /// Prerequisite: Successful call to parse or compile. - std::string const& getUserDocumentation(); - /// Returns a string representing the contract's developer documentation in JSON. + /// Returns a string representing the contract's documentation in JSON. /// Prerequisite: Successful call to parse or compile. - std::string const& getDevDocumentation(); + /// @param type The type of the documentation to get. + /// Can be one of 3 types defined at @c documentation_type + std::string const* getJsonDocumentation(enum documentation_type type); /// Returns the previously used scanner, useful for counting lines during error reporting. Scanner const& getScanner() const { return *m_scanner; } @@ -82,10 +89,11 @@ private: std::shared_ptr m_globalContext; std::shared_ptr m_contractASTNode; bool m_parseSuccessful; - std::string m_interface; - std::string m_userDocumentation; - std::string m_devDocumentation; + std::unique_ptr m_interface; + std::unique_ptr m_userDocumentation; + std::unique_ptr m_devDocumentation; std::shared_ptr m_compiler; + std::shared_ptr m_interfaceHandler; bytes m_bytecode; }; diff --git a/libsolidity/InterfaceHandler.cpp b/libsolidity/InterfaceHandler.cpp new file mode 100644 index 000000000..c0e07280e --- /dev/null +++ b/libsolidity/InterfaceHandler.cpp @@ -0,0 +1,88 @@ +#include +#include +#include + +namespace dev { +namespace solidity { + +InterfaceHandler::InterfaceHandler() +{ +} + +std::unique_ptr InterfaceHandler::getDocumentation(std::shared_ptr _contractDef, + enum documentation_type _type) +{ + switch(_type) + { + case NATSPEC_USER: + return getUserDocumentation(_contractDef); + case NATSPEC_DEV: + return getDevDocumentation(_contractDef); + case ABI_INTERFACE: + return getABIInterface(_contractDef); + } + + BOOST_THROW_EXCEPTION(CompilerError() << errinfo_comment("Internal error")); + return nullptr; +} + +std::unique_ptr InterfaceHandler::getABIInterface(std::shared_ptr _contractDef) +{ + Json::Value methods(Json::arrayValue); + + std::vector exportedFunctions = _contractDef->getInterfaceFunctions(); + for (FunctionDefinition const* f: exportedFunctions) + { + Json::Value method; + Json::Value inputs(Json::arrayValue); + Json::Value outputs(Json::arrayValue); + + auto streamVariables = [](std::vector> const& _vars) + { + Json::Value params(Json::arrayValue); + for (ASTPointer const& var: _vars) + { + Json::Value input; + input["name"] = var->getName(); + input["type"] = var->getType()->toString(); + params.append(input); + } + return params; + }; + + method["name"] = f->getName(); + method["inputs"] = streamVariables(f->getParameters()); + method["outputs"] = streamVariables(f->getReturnParameters()); + methods.append(method); + } + return std::unique_ptr(new std::string(m_writer.write(methods))); +} + +std::unique_ptr InterfaceHandler::getUserDocumentation(std::shared_ptr _contractDef) +{ + Json::Value doc; + Json::Value methods(Json::objectValue); + + for (FunctionDefinition const* f: _contractDef->getInterfaceFunctions()) + { + Json::Value user; + auto strPtr = f->getDocumentation(); + if (strPtr) + { + user["notice"] = Json::Value(*strPtr); + methods[f->getName()] = user; + } + } + doc["methods"] = methods; + + return std::unique_ptr(new std::string(m_writer.write(doc))); +} + +std::unique_ptr InterfaceHandler::getDevDocumentation(std::shared_ptr _contractDef) +{ + //TODO + return nullptr; +} + +} //solidity NS +} // dev NS diff --git a/libsolidity/InterfaceHandler.h b/libsolidity/InterfaceHandler.h new file mode 100644 index 000000000..ed6c8ba44 --- /dev/null +++ b/libsolidity/InterfaceHandler.h @@ -0,0 +1,74 @@ +/* + This file is part of cpp-ethereum. + + cpp-ethereum is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + cpp-ethereum is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with cpp-ethereum. If not, see . +*/ +/** + * @author Lefteris + * @date 2014 + * Takes the parsed AST and produces the Natspec + * documentation and the ABI interface + * https://github.com/ethereum/wiki/wiki/Ethereum-Natural-Specification-Format + * + * Can generally deal with JSON files + */ + +#pragma once + +#include +#include +#include + +namespace dev { +namespace solidity { + +// Forward declarations +class ContractDefinition; +enum documentation_type: unsigned short; + +class InterfaceHandler +{ +public: + InterfaceHandler(); + + /// Get the given type of documentation + /// @param _contractDef The contract definition + /// @param _type The type of the documentation. Can be one of the + /// types provided by @c documentation_type + /// @return A unique pointer contained string with the json + /// representation of provided type + std::unique_ptr getDocumentation(std::shared_ptr _contractDef, + enum documentation_type _type); + /// Get the ABI Interface of the contract + /// @param _contractDef The contract definition + /// @return A unique pointer contained string with the json + /// representation of the contract's ABI Interface + std::unique_ptr getABIInterface(std::shared_ptr _contractDef); + /// Get the User documentation of the contract + /// @param _contractDef The contract definition + /// @return A unique pointer contained string with the json + /// representation of the contract's user documentation + std::unique_ptr getUserDocumentation(std::shared_ptr _contractDef); + /// Get the Developer's documentation of the contract + /// @param _contractDef The contract definition + /// @return A unique pointer contained string with the json + /// representation of the contract's developer documentation + std::unique_ptr getDevDocumentation(std::shared_ptr _contractDef); + +private: + Json::StyledWriter m_writer; +}; + +} //solidity NS +} // dev NS diff --git a/solc/main.cpp b/solc/main.cpp index 29239a717..daeb2707c 100644 --- a/solc/main.cpp +++ b/solc/main.cpp @@ -135,8 +135,9 @@ int main(int argc, char** argv) cout << "Opcodes:" << endl; cout << eth::disassemble(compiler.getBytecode()) << endl; cout << "Binary: " << toHex(compiler.getBytecode()) << endl; - cout << "Interface specification: " << compiler.getInterface() << endl; - cout << "Natspec user documentation: " << compiler.getUserDocumentation() << endl; + cout << "Interface specification: " << compiler.getJsonDocumentation(ABI_INTERFACE) << endl; + cout << "Natspec user documentation: " << compiler.getJsonDocumentation(NATSPEC_USER) << endl; + cout << "Natspec developer documentation: " << compiler.getJsonDocumentation(NATSPEC_DEV) << endl; return 0; } diff --git a/test/solidityJSONInterfaceTest.cpp b/test/solidityJSONInterfaceTest.cpp index f46a3ad36..8fe0ea653 100644 --- a/test/solidityJSONInterfaceTest.cpp +++ b/test/solidityJSONInterfaceTest.cpp @@ -50,7 +50,7 @@ public: msg += *extra; BOOST_FAIL(msg); } - std::string generatedInterfaceString = m_compilerStack.getInterface(); + std::string generatedInterfaceString = *m_compilerStack.getJsonDocumentation(ABI_INTERFACE); Json::Value generatedInterface; m_reader.parse(generatedInterfaceString, generatedInterface); Json::Value expectedInterface; diff --git a/test/solidityNatspecJSON.cpp b/test/solidityNatspecJSON.cpp index f729e4ff8..9e24d23ed 100644 --- a/test/solidityNatspecJSON.cpp +++ b/test/solidityNatspecJSON.cpp @@ -55,9 +55,9 @@ public: } if (_userDocumentation) - generatedDocumentationString = m_compilerStack.getUserDocumentation(); + generatedDocumentationString = *m_compilerStack.getJsonDocumentation(NATSPEC_USER); else - generatedDocumentationString = m_compilerStack.getDevDocumentation(); + generatedDocumentationString = *m_compilerStack.getJsonDocumentation(NATSPEC_DEV); Json::Value generatedDocumentation; m_reader.parse(generatedDocumentationString, generatedDocumentation); Json::Value expectedDocumentation; From eeb186c834aaf30ba63a10585cc9c66ae33fcd6c Mon Sep 17 00:00:00 2001 From: Lefteris Karapetsas Date: Wed, 3 Dec 2014 17:46:04 +0100 Subject: [PATCH 09/27] Work in progress for parsing natspec doxytags --- libsolidity/InterfaceHandler.cpp | 69 ++++++++++++++++++++++++++++++-- libsolidity/InterfaceHandler.h | 11 ++++- 2 files changed, 75 insertions(+), 5 deletions(-) diff --git a/libsolidity/InterfaceHandler.cpp b/libsolidity/InterfaceHandler.cpp index c0e07280e..a2c52c1f0 100644 --- a/libsolidity/InterfaceHandler.cpp +++ b/libsolidity/InterfaceHandler.cpp @@ -5,12 +5,14 @@ namespace dev { namespace solidity { +/* -- public -- */ + InterfaceHandler::InterfaceHandler() { } std::unique_ptr InterfaceHandler::getDocumentation(std::shared_ptr _contractDef, - enum documentation_type _type) + enum documentation_type _type) { switch(_type) { @@ -80,8 +82,69 @@ std::unique_ptr InterfaceHandler::getUserDocumentation(std::shared_ std::unique_ptr InterfaceHandler::getDevDocumentation(std::shared_ptr _contractDef) { - //TODO - return nullptr; + Json::Value doc; + Json::Value methods(Json::objectValue); + + for (FunctionDefinition const* f: _contractDef->getInterfaceFunctions()) + { + Json::Value method; + auto strPtr = f->getDocumentation(); + if (strPtr) + { + m_dev.clear(); + parseDocString(*strPtr); + + method["dev"] = Json::Value(m_dev); + methods[f->getName()] = method; + } + } + doc["methods"] = methods; + + return std::unique_ptr(new std::string(m_writer.write(doc))); +} + +/* -- private -- */ +size_t InterfaceHandler::parseDocTag(std::string const& _string, std::string const& _tag, size_t _pos) +{ + size_t nlPos = _pos; + if (_tag == "dev") + { + nlPos = _string.find("\n", _pos); + m_dev += _string.substr(_pos, + nlPos == std::string::npos ? + _string.length() : + nlPos - _pos); + } + else if (_tag == "notice") + { + nlPos = _string.find("\n", _pos); + m_notice += _string.substr(_pos, + nlPos == std::string::npos ? + _string.length() : + nlPos - _pos); + } + else + { + //TODO: Some form of warning + } + + return nlPos; +} + +void InterfaceHandler::parseDocString(std::string const& _string, size_t _startPos) +{ + size_t pos2; + size_t pos1 = _string.find("@", _startPos); + + if (pos1 == std::string::npos) + return; // no doxytags found + + pos2 = _string.find(" ", pos1); + if (pos2 == std::string::npos) + return; //no end of tag found + + size_t newPos = parseDocTag(_string, _string.substr(pos1 + 1, pos2 - pos1), pos2); + parseDocString(_string, newPos); } } //solidity NS diff --git a/libsolidity/InterfaceHandler.h b/libsolidity/InterfaceHandler.h index ed6c8ba44..125ecda4b 100644 --- a/libsolidity/InterfaceHandler.h +++ b/libsolidity/InterfaceHandler.h @@ -20,7 +20,7 @@ * Takes the parsed AST and produces the Natspec * documentation and the ABI interface * https://github.com/ethereum/wiki/wiki/Ethereum-Natural-Specification-Format - * + * * Can generally deal with JSON files */ @@ -44,7 +44,7 @@ public: /// Get the given type of documentation /// @param _contractDef The contract definition - /// @param _type The type of the documentation. Can be one of the + /// @param _type The type of the documentation. Can be one of the /// types provided by @c documentation_type /// @return A unique pointer contained string with the json /// representation of provided type @@ -67,7 +67,14 @@ public: std::unique_ptr getDevDocumentation(std::shared_ptr _contractDef); private: + void parseDocString(std::string const& _string, size_t _startPos = 0); + size_t parseDocTag(std::string const& _string, std::string const& _tag, size_t _pos); + Json::StyledWriter m_writer; + + // internal state + std::string m_notice; + std::string m_dev; }; } //solidity NS From 2f16217574b2d610967ee094e209065d50531f90 Mon Sep 17 00:00:00 2001 From: Lefteris Karapetsas Date: Thu, 4 Dec 2014 01:27:38 +0100 Subject: [PATCH 10/27] Styling in libsolidity's InterfaceHandler --- libsolidity/CompilerStack.cpp | 4 ++-- libsolidity/CompilerStack.h | 4 ++-- libsolidity/InterfaceHandler.cpp | 8 ++++---- libsolidity/InterfaceHandler.h | 4 ++-- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/libsolidity/CompilerStack.cpp b/libsolidity/CompilerStack.cpp index 6f80d245c..3e9791bd2 100644 --- a/libsolidity/CompilerStack.cpp +++ b/libsolidity/CompilerStack.cpp @@ -84,12 +84,12 @@ void CompilerStack::streamAssembly(ostream& _outStream) m_compiler->streamAssembly(_outStream); } -std::string const* CompilerStack::getJsonDocumentation(enum documentation_type _type) +std::string const* CompilerStack::getJsonDocumentation(enum documentationType _type) { if (!m_parseSuccessful) BOOST_THROW_EXCEPTION(CompilerError() << errinfo_comment("Parsing was not successful.")); - auto createOrReturnDoc = [&, this](std::unique_ptr& _doc) + auto createOrReturnDoc = [this, _type](std::unique_ptr& _doc) { if(!_doc) { diff --git a/libsolidity/CompilerStack.h b/libsolidity/CompilerStack.h index 7dc86e2be..511951291 100644 --- a/libsolidity/CompilerStack.h +++ b/libsolidity/CompilerStack.h @@ -37,7 +37,7 @@ class Compiler; class GlobalContext; class InterfaceHandler; -enum documentation_type : unsigned short +enum documentationType: unsigned short { NATSPEC_USER = 1, NATSPEC_DEV, @@ -74,7 +74,7 @@ public: /// Prerequisite: Successful call to parse or compile. /// @param type The type of the documentation to get. /// Can be one of 3 types defined at @c documentation_type - std::string const* getJsonDocumentation(enum documentation_type type); + std::string const* getJsonDocumentation(enum documentationType type); /// Returns the previously used scanner, useful for counting lines during error reporting. Scanner const& getScanner() const { return *m_scanner; } diff --git a/libsolidity/InterfaceHandler.cpp b/libsolidity/InterfaceHandler.cpp index a2c52c1f0..61570b8d7 100644 --- a/libsolidity/InterfaceHandler.cpp +++ b/libsolidity/InterfaceHandler.cpp @@ -12,7 +12,7 @@ InterfaceHandler::InterfaceHandler() } std::unique_ptr InterfaceHandler::getDocumentation(std::shared_ptr _contractDef, - enum documentation_type _type) + enum documentationType _type) { switch(_type) { @@ -39,7 +39,7 @@ std::unique_ptr InterfaceHandler::getABIInterface(std::shared_ptr> const& _vars) + auto populateParameters = [](std::vector> const& _vars) { Json::Value params(Json::arrayValue); for (ASTPointer const& var: _vars) @@ -53,8 +53,8 @@ std::unique_ptr InterfaceHandler::getABIInterface(std::shared_ptrgetName(); - method["inputs"] = streamVariables(f->getParameters()); - method["outputs"] = streamVariables(f->getReturnParameters()); + method["inputs"] = populateParameters(f->getParameters()); + method["outputs"] = populateParameters(f->getReturnParameters()); methods.append(method); } return std::unique_ptr(new std::string(m_writer.write(methods))); diff --git a/libsolidity/InterfaceHandler.h b/libsolidity/InterfaceHandler.h index 125ecda4b..5c8bf5bca 100644 --- a/libsolidity/InterfaceHandler.h +++ b/libsolidity/InterfaceHandler.h @@ -35,7 +35,7 @@ namespace solidity { // Forward declarations class ContractDefinition; -enum documentation_type: unsigned short; +enum documentationType: unsigned short; class InterfaceHandler { @@ -49,7 +49,7 @@ public: /// @return A unique pointer contained string with the json /// representation of provided type std::unique_ptr getDocumentation(std::shared_ptr _contractDef, - enum documentation_type _type); + enum documentationType _type); /// Get the ABI Interface of the contract /// @param _contractDef The contract definition /// @return A unique pointer contained string with the json From 5fe11335834395d5b8033a5b24eb6ae7c225e4f2 Mon Sep 17 00:00:00 2001 From: Lefteris Karapetsas Date: Thu, 4 Dec 2014 09:42:38 +0100 Subject: [PATCH 11/27] Parsing notice and dev doxytags. - Only initial work done. Still need to refine the logic and incorporate all the other types of tags. - Added/Modified some tests - Work in progress --- libsolidity/InterfaceHandler.cpp | 90 ++++++++++++++++++++++++-------- libsolidity/InterfaceHandler.h | 8 +++ test/solidityNatspecJSON.cpp | 58 ++++++++++++++++---- 3 files changed, 122 insertions(+), 34 deletions(-) diff --git a/libsolidity/InterfaceHandler.cpp b/libsolidity/InterfaceHandler.cpp index 61570b8d7..e7da5e6d5 100644 --- a/libsolidity/InterfaceHandler.cpp +++ b/libsolidity/InterfaceHandler.cpp @@ -9,6 +9,7 @@ namespace solidity { InterfaceHandler::InterfaceHandler() { + m_lastTag = DOCTAG_NONE; } std::unique_ptr InterfaceHandler::getDocumentation(std::shared_ptr _contractDef, @@ -71,7 +72,9 @@ std::unique_ptr InterfaceHandler::getUserDocumentation(std::shared_ auto strPtr = f->getDocumentation(); if (strPtr) { - user["notice"] = Json::Value(*strPtr); + m_notice.clear(); + parseDocString(*strPtr); + user["notice"] = Json::Value(m_notice); methods[f->getName()] = user; } } @@ -94,7 +97,7 @@ std::unique_ptr InterfaceHandler::getDevDocumentation(std::shared_p m_dev.clear(); parseDocString(*strPtr); - method["dev"] = Json::Value(m_dev); + method["details"] = Json::Value(m_dev); methods[f->getName()] = method; } } @@ -106,26 +109,55 @@ std::unique_ptr InterfaceHandler::getDevDocumentation(std::shared_p /* -- private -- */ size_t InterfaceHandler::parseDocTag(std::string const& _string, std::string const& _tag, size_t _pos) { + //TODO: This is pretty naive at the moment. e.g. need to check for + // '@' between _pos and \n, remove redundancy e.t.c. size_t nlPos = _pos; - if (_tag == "dev") - { - nlPos = _string.find("\n", _pos); - m_dev += _string.substr(_pos, - nlPos == std::string::npos ? - _string.length() : - nlPos - _pos); - } - else if (_tag == "notice") + if (m_lastTag == DOCTAG_NONE || _tag != "") { - nlPos = _string.find("\n", _pos); - m_notice += _string.substr(_pos, - nlPos == std::string::npos ? - _string.length() : - nlPos - _pos); + if (_tag == "dev") + { + nlPos = _string.find("\n", _pos); + m_dev += _string.substr(_pos, + nlPos == std::string::npos ? + _string.length() : + nlPos - _pos); + m_lastTag = DOCTAG_DEV; + } + else if (_tag == "notice") + { + nlPos = _string.find("\n", _pos); + m_notice += _string.substr(_pos, + nlPos == std::string::npos ? + _string.length() : + nlPos - _pos); + m_lastTag = DOCTAG_NOTICE; + } + else + { + //TODO: Some form of warning + } } else { - //TODO: Some form of warning + switch(m_lastTag) + { + case DOCTAG_DEV: + nlPos = _string.find("\n", _pos); + m_dev += _string.substr(_pos, + nlPos == std::string::npos ? + _string.length() : + nlPos - _pos); + break; + case DOCTAG_NOTICE: + nlPos = _string.find("\n", _pos); + m_notice += _string.substr(_pos, + nlPos == std::string::npos ? + _string.length() : + nlPos - _pos); + break; + default: + break; + } } return nlPos; @@ -134,16 +166,28 @@ size_t InterfaceHandler::parseDocTag(std::string const& _string, std::string con void InterfaceHandler::parseDocString(std::string const& _string, size_t _startPos) { size_t pos2; + size_t newPos = _startPos; size_t pos1 = _string.find("@", _startPos); - if (pos1 == std::string::npos) - return; // no doxytags found + if (pos1 != std::string::npos) + { + // we found a tag + pos2 = _string.find(" ", pos1); + if (pos2 == std::string::npos) + { + BOOST_THROW_EXCEPTION(CompilerError() << errinfo_comment("End of tag not found")); + return; //no end of tag found + } - pos2 = _string.find(" ", pos1); - if (pos2 == std::string::npos) - return; //no end of tag found + newPos = parseDocTag(_string, _string.substr(pos1 + 1, pos2 - pos1 - 1), pos2 + 1); + } + else + { + newPos = parseDocTag(_string, "", _startPos + 1); + } - size_t newPos = parseDocTag(_string, _string.substr(pos1 + 1, pos2 - pos1), pos2); + if (newPos == std::string::npos) + return; // EOS parseDocString(_string, newPos); } diff --git a/libsolidity/InterfaceHandler.h b/libsolidity/InterfaceHandler.h index 5c8bf5bca..6f2f2937d 100644 --- a/libsolidity/InterfaceHandler.h +++ b/libsolidity/InterfaceHandler.h @@ -37,6 +37,13 @@ namespace solidity { class ContractDefinition; enum documentationType: unsigned short; +enum docTagType +{ + DOCTAG_NONE = 0, + DOCTAG_DEV, + DOCTAG_NOTICE, +}; + class InterfaceHandler { public: @@ -73,6 +80,7 @@ private: Json::StyledWriter m_writer; // internal state + enum docTagType m_lastTag; std::string m_notice; std::string m_dev; }; diff --git a/test/solidityNatspecJSON.cpp b/test/solidityNatspecJSON.cpp index 9e24d23ed..d74aa8ab9 100644 --- a/test/solidityNatspecJSON.cpp +++ b/test/solidityNatspecJSON.cpp @@ -77,22 +77,59 @@ BOOST_FIXTURE_TEST_SUITE(SolidityNatspecJSON, DocumentationChecker) BOOST_AUTO_TEST_CASE(user_basic_test) { char const* sourceCode = "contract test {\n" - " /// Multiplies `a` by 7\n" + " /// @notice Multiplies `a` by 7\n" " function mul(uint a) returns(uint d) { return a * 7; }\n" "}\n"; char const* natspec = "{" "\"methods\":{" - " \"mul\":{ \"notice\": \" Multiplies `a` by 7\"}" + " \"mul\":{ \"notice\": \"Multiplies `a` by 7\"}" "}}"; checkNatspec(sourceCode, natspec, true); } +BOOST_AUTO_TEST_CASE(dev_basic_test) +{ + char const* sourceCode = "contract test {\n" + " /// @dev Multiplies a number by 7\n" + " function mul(uint a) returns(uint d) { return a * 7; }\n" + "}\n"; + + char const* natspec = "{" + "\"methods\":{" + " \"mul\":{ \"details\": \"Multiplies a number by 7\"}" + "}}"; + + checkNatspec(sourceCode, natspec, false); +} + +BOOST_AUTO_TEST_CASE(dev_and_user_basic_test) +{ + char const* sourceCode = "contract test {\n" + " /// @notice Multiplies `a` by 7\n" + " /// @dev Multiplies a number by 7\n" + " function mul(uint a) returns(uint d) { return a * 7; }\n" + "}\n"; + + char const* devNatspec = "{" + "\"methods\":{" + " \"mul\":{ \"details\": \"Multiplies a number by 7\"}" + "}}"; + + char const* userNatspec = "{" + "\"methods\":{" + " \"mul\":{ \"notice\": \"Multiplies `a` by 7\"}" + "}}"; + + checkNatspec(sourceCode, devNatspec, false); + checkNatspec(sourceCode, userNatspec, true); +} + BOOST_AUTO_TEST_CASE(user_multiline_comment) { char const* sourceCode = "contract test {\n" - " /// Multiplies `a` by 7\n" + " /// @notice Multiplies `a` by 7\n" " /// and then adds `b`\n" " function mul_and_add(uint a, uint256 b) returns(uint256 d)\n" " {\n" @@ -102,7 +139,7 @@ BOOST_AUTO_TEST_CASE(user_multiline_comment) char const* natspec = "{" "\"methods\":{" - " \"mul_and_add\":{ \"notice\": \" Multiplies `a` by 7\n and then adds `b`\"}" + " \"mul_and_add\":{ \"notice\": \"Multiplies `a` by 7 and then adds `b`\"}" "}}"; checkNatspec(sourceCode, natspec, true); @@ -111,19 +148,18 @@ BOOST_AUTO_TEST_CASE(user_multiline_comment) BOOST_AUTO_TEST_CASE(user_multiple_functions) { char const* sourceCode = "contract test {\n" - " /// Multiplies `a` by 7\n" - " /// and then adds `b`\n" + " /// @notice Multiplies `a` by 7 and then adds `b`\n" " function mul_and_add(uint a, uint256 b) returns(uint256 d)\n" " {\n" " return (a * 7) + b;\n" " }\n" "\n" - " /// Divides `input` by `div`\n" + " /// @notice Divides `input` by `div`\n" " function divide(uint input, uint div) returns(uint d)\n" " {\n" " return input / div;\n" " }\n" - " /// Subtracts 3 from `input`\n" + " /// @notice Subtracts 3 from `input`\n" " function sub(int input) returns(int d)\n" " {\n" " return input - 3;\n" @@ -132,9 +168,9 @@ BOOST_AUTO_TEST_CASE(user_multiple_functions) char const* natspec = "{" "\"methods\":{" - " \"mul_and_add\":{ \"notice\": \" Multiplies `a` by 7\n and then adds `b`\"}," - " \"divide\":{ \"notice\": \" Divides `input` by `div`\"}," - " \"sub\":{ \"notice\": \" Subtracts 3 from `input`\"}" + " \"mul_and_add\":{ \"notice\": \"Multiplies `a` by 7 and then adds `b`\"}," + " \"divide\":{ \"notice\": \"Divides `input` by `div`\"}," + " \"sub\":{ \"notice\": \"Subtracts 3 from `input`\"}" "}}"; checkNatspec(sourceCode, natspec, true); From 112c583ddb3b43a7260156ab65694e55aac20130 Mon Sep 17 00:00:00 2001 From: Lefteris Karapetsas Date: Thu, 4 Dec 2014 17:19:47 +0100 Subject: [PATCH 12/27] Natspec parsing @param doctags - Plus additional work on generally parsing doctags. One important missing feature is to parse a tag midline - Adding more tests --- libsolidity/InterfaceHandler.cpp | 158 ++++++++++++++++++++++--------- libsolidity/InterfaceHandler.h | 9 ++ test/solidityNatspecJSON.cpp | 117 +++++++++++++++++++---- 3 files changed, 225 insertions(+), 59 deletions(-) diff --git a/libsolidity/InterfaceHandler.cpp b/libsolidity/InterfaceHandler.cpp index e7da5e6d5..222711e6a 100644 --- a/libsolidity/InterfaceHandler.cpp +++ b/libsolidity/InterfaceHandler.cpp @@ -1,3 +1,4 @@ + #include #include #include @@ -72,7 +73,7 @@ std::unique_ptr InterfaceHandler::getUserDocumentation(std::shared_ auto strPtr = f->getDocumentation(); if (strPtr) { - m_notice.clear(); + resetUser(); parseDocString(*strPtr); user["notice"] = Json::Value(m_notice); methods[f->getName()] = user; @@ -94,10 +95,16 @@ std::unique_ptr InterfaceHandler::getDevDocumentation(std::shared_p auto strPtr = f->getDocumentation(); if (strPtr) { - m_dev.clear(); + resetDev(); parseDocString(*strPtr); method["details"] = Json::Value(m_dev); + Json::Value params(Json::objectValue); + for (auto const& pair: m_params) + { + params[pair.first] = pair.second; + } + method["params"] = params; methods[f->getName()] = method; } } @@ -107,86 +114,151 @@ std::unique_ptr InterfaceHandler::getDevDocumentation(std::shared_p } /* -- private -- */ +void InterfaceHandler::resetUser() +{ + m_notice.clear(); +} + +void InterfaceHandler::resetDev() +{ + m_dev.clear(); + m_params.clear(); +} + +size_t skipLineOrEOS(std::string const& _string, size_t _nlPos) +{ + return (_nlPos == std::string::npos) ? _string.length() : _nlPos + 1; +} + +size_t InterfaceHandler::parseDocTagLine(std::string const& _string, + std::string& _tagString, + size_t _pos, + enum docTagType _tagType) +{ + size_t nlPos = _string.find("\n", _pos); + _tagString += _string.substr(_pos, + nlPos == std::string::npos ? + _string.length() : + nlPos - _pos); + m_lastTag = _tagType; + return skipLineOrEOS(_string, nlPos); +} + +size_t InterfaceHandler::parseDocTagParam(std::string const& _string, size_t _startPos) +{ + // find param name + size_t currPos = _string.find(" ", _startPos); + if (currPos == std::string::npos) + { + BOOST_THROW_EXCEPTION(CompilerError() << errinfo_comment("End of param name not found")); + return currPos; //no end of tag found + } + + auto paramName = _string.substr(_startPos, currPos - _startPos); + + currPos += 1; + size_t nlPos = _string.find("\n", currPos); + auto paramDesc = _string.substr(currPos, + nlPos == std::string::npos ? + _string.length() : + nlPos - currPos); + + m_params.push_back(std::make_pair(paramName, paramDesc)); + + m_lastTag = DOCTAG_PARAM; + return skipLineOrEOS(_string, nlPos); +} + +size_t InterfaceHandler::appendDocTagParam(std::string const& _string, size_t _startPos) +{ + // Should never be called with an empty vector + assert(!m_params.empty()); + + auto pair = m_params.back(); + size_t nlPos = _string.find("\n", _startPos); + pair.second += _string.substr(_startPos, + nlPos == std::string::npos ? + _string.length() : + nlPos - _startPos); + + m_params.at(m_params.size() - 1) = pair; + + return skipLineOrEOS(_string, nlPos); +} + size_t InterfaceHandler::parseDocTag(std::string const& _string, std::string const& _tag, size_t _pos) { - //TODO: This is pretty naive at the moment. e.g. need to check for - // '@' between _pos and \n, remove redundancy e.t.c. + //TODO: need to check for @(start of a tag) between here and the end of line + // for all cases size_t nlPos = _pos; if (m_lastTag == DOCTAG_NONE || _tag != "") { if (_tag == "dev") - { - nlPos = _string.find("\n", _pos); - m_dev += _string.substr(_pos, - nlPos == std::string::npos ? - _string.length() : - nlPos - _pos); - m_lastTag = DOCTAG_DEV; - } + nlPos = parseDocTagLine(_string, m_dev, _pos, DOCTAG_DEV); else if (_tag == "notice") - { - nlPos = _string.find("\n", _pos); - m_notice += _string.substr(_pos, - nlPos == std::string::npos ? - _string.length() : - nlPos - _pos); - m_lastTag = DOCTAG_NOTICE; - } + nlPos = parseDocTagLine(_string, m_notice, _pos, DOCTAG_NOTICE); + else if (_tag == "param") + nlPos = parseDocTagParam(_string, _pos); else { //TODO: Some form of warning } } else + appendDocTag(_string, _pos); + + return nlPos; +} + +size_t InterfaceHandler::appendDocTag(std::string const& _string, size_t _startPos) +{ + size_t newPos = _startPos; + switch(m_lastTag) { - switch(m_lastTag) - { case DOCTAG_DEV: - nlPos = _string.find("\n", _pos); - m_dev += _string.substr(_pos, - nlPos == std::string::npos ? - _string.length() : - nlPos - _pos); + m_dev += " "; + newPos = parseDocTagLine(_string, m_dev, _startPos, DOCTAG_DEV); break; case DOCTAG_NOTICE: - nlPos = _string.find("\n", _pos); - m_notice += _string.substr(_pos, - nlPos == std::string::npos ? - _string.length() : - nlPos - _pos); + m_notice += " "; + newPos = parseDocTagLine(_string, m_notice, _startPos, DOCTAG_NOTICE); + break; + case DOCTAG_PARAM: + newPos = appendDocTagParam(_string, _startPos); break; default: break; } - } - - return nlPos; + return newPos; } void InterfaceHandler::parseDocString(std::string const& _string, size_t _startPos) { size_t pos2; size_t newPos = _startPos; - size_t pos1 = _string.find("@", _startPos); + size_t tagPos = _string.find("@", _startPos); + size_t nlPos = _string.find("\n", _startPos); - if (pos1 != std::string::npos) + if (tagPos != std::string::npos && tagPos < nlPos) { // we found a tag - pos2 = _string.find(" ", pos1); + pos2 = _string.find(" ", tagPos); if (pos2 == std::string::npos) { BOOST_THROW_EXCEPTION(CompilerError() << errinfo_comment("End of tag not found")); return; //no end of tag found } - newPos = parseDocTag(_string, _string.substr(pos1 + 1, pos2 - pos1 - 1), pos2 + 1); + newPos = parseDocTag(_string, _string.substr(tagPos + 1, pos2 - tagPos - 1), pos2 + 1); } - else + else if (m_lastTag != DOCTAG_NONE) // continuation of the previous tag + newPos = appendDocTag(_string, _startPos + 1); + else // skip the line if a newline was found { - newPos = parseDocTag(_string, "", _startPos + 1); + if (newPos != std::string::npos) + newPos = nlPos + 1; } - - if (newPos == std::string::npos) + if (newPos == std::string::npos || newPos == _string.length()) return; // EOS parseDocString(_string, newPos); } diff --git a/libsolidity/InterfaceHandler.h b/libsolidity/InterfaceHandler.h index 6f2f2937d..2a70af950 100644 --- a/libsolidity/InterfaceHandler.h +++ b/libsolidity/InterfaceHandler.h @@ -42,6 +42,7 @@ enum docTagType DOCTAG_NONE = 0, DOCTAG_DEV, DOCTAG_NOTICE, + DOCTAG_PARAM }; class InterfaceHandler @@ -74,7 +75,14 @@ public: std::unique_ptr getDevDocumentation(std::shared_ptr _contractDef); private: + void resetUser(); + void resetDev(); + + size_t parseDocTagLine(std::string const& _string, std::string& _tagString, size_t _pos, enum docTagType _tagType); + size_t parseDocTagParam(std::string const& _string, size_t _startPos); + size_t appendDocTagParam(std::string const& _string, size_t _startPos); void parseDocString(std::string const& _string, size_t _startPos = 0); + size_t appendDocTag(std::string const& _string, size_t _startPos); size_t parseDocTag(std::string const& _string, std::string const& _tag, size_t _pos); Json::StyledWriter m_writer; @@ -83,6 +91,7 @@ private: enum docTagType m_lastTag; std::string m_notice; std::string m_dev; + std::vector> m_params; }; } //solidity NS diff --git a/test/solidityNatspecJSON.cpp b/test/solidityNatspecJSON.cpp index d74aa8ab9..5894f3b04 100644 --- a/test/solidityNatspecJSON.cpp +++ b/test/solidityNatspecJSON.cpp @@ -89,21 +89,6 @@ BOOST_AUTO_TEST_CASE(user_basic_test) checkNatspec(sourceCode, natspec, true); } -BOOST_AUTO_TEST_CASE(dev_basic_test) -{ - char const* sourceCode = "contract test {\n" - " /// @dev Multiplies a number by 7\n" - " function mul(uint a) returns(uint d) { return a * 7; }\n" - "}\n"; - - char const* natspec = "{" - "\"methods\":{" - " \"mul\":{ \"details\": \"Multiplies a number by 7\"}" - "}}"; - - checkNatspec(sourceCode, natspec, false); -} - BOOST_AUTO_TEST_CASE(dev_and_user_basic_test) { char const* sourceCode = "contract test {\n" @@ -114,7 +99,11 @@ BOOST_AUTO_TEST_CASE(dev_and_user_basic_test) char const* devNatspec = "{" "\"methods\":{" - " \"mul\":{ \"details\": \"Multiplies a number by 7\"}" + " \"mul\":{ \n" + " \"details\": \"Multiplies a number by 7\",\n" + " \"params\": {}\n" + " }\n" + " }\n" "}}"; char const* userNatspec = "{" @@ -186,6 +175,102 @@ BOOST_AUTO_TEST_CASE(user_empty_contract) checkNatspec(sourceCode, natspec, true); } +BOOST_AUTO_TEST_CASE(dev_multiple_params) +{ + char const* sourceCode = "contract test {\n" + " /// @dev Multiplies a number by 7 and adds second parameter\n" + " /// @param a Documentation for the first parameter\n" + " /// @param second Documentation for the second parameter\n" + " function mul(uint a, uint second) returns(uint d) { return a * 7 + second; }\n" + "}\n"; + + char const* natspec = "{" + "\"methods\":{" + " \"mul\":{ \n" + " \"details\": \"Multiplies a number by 7 and adds second parameter\",\n" + " \"params\": {\n" + " \"a\": \"Documentation for the first parameter\",\n" + " \"second\": \"Documentation for the second parameter\"\n" + " }\n" + " }\n" + "}}"; + + checkNatspec(sourceCode, natspec, false); +} + +BOOST_AUTO_TEST_CASE(dev_mutiline_param_description) +{ + char const* sourceCode = "contract test {\n" + " /// @dev Multiplies a number by 7 and adds second parameter\n" + " /// @param a Documentation for the first parameter starts here.\n" + " /// Since it's a really complicated parameter we need 2 lines\n" + " /// @param second Documentation for the second parameter\n" + " function mul(uint a, uint second) returns(uint d) { return a * 7 + second; }\n" + "}\n"; + + char const* natspec = "{" + "\"methods\":{" + " \"mul\":{ \n" + " \"details\": \"Multiplies a number by 7 and adds second parameter\",\n" + " \"params\": {\n" + " \"a\": \"Documentation for the first parameter starts here.Since it's a really complicated parameter we need 2 lines\",\n" + " \"second\": \"Documentation for the second parameter\"\n" + " }\n" + " }\n" + "}}"; + + checkNatspec(sourceCode, natspec, false); +} + +BOOST_AUTO_TEST_CASE(dev_multiple_functions) +{ + char const* sourceCode = "contract test {\n" + " /// @dev Multiplies a number by 7 and adds second parameter\n" + " /// @param a Documentation for the first parameter\n" + " /// @param second Documentation for the second parameter\n" + " function mul(uint a, uint second) returns(uint d) { return a * 7 + second; }\n" + " \n" + " /// @dev Divides 2 numbers\n" + " /// @param input Documentation for the input parameter\n" + " /// @param div Documentation for the div parameter\n" + " function divide(uint input, uint div) returns(uint d)\n" + " {\n" + " return input / div;\n" + " }\n" + " /// @dev Subtracts 3 from `input`\n" + " /// @param input Documentation for the input parameter\n" + " function sub(int input) returns(int d)\n" + " {\n" + " return input - 3;\n" + " }\n" + "}\n"; + + char const* natspec = "{" + "\"methods\":{" + " \"mul\":{ \n" + " \"details\": \"Multiplies a number by 7 and adds second parameter\",\n" + " \"params\": {\n" + " \"a\": \"Documentation for the first parameter\",\n" + " \"second\": \"Documentation for the second parameter\"\n" + " }\n" + " },\n" + " \"divide\":{ \n" + " \"details\": \"Divides 2 numbers\",\n" + " \"params\": {\n" + " \"input\": \"Documentation for the input parameter\",\n" + " \"div\": \"Documentation for the div parameter\"\n" + " }\n" + " },\n" + " \"sub\":{ \n" + " \"details\": \"Subtracts 3 from `input`\",\n" + " \"params\": {\n" + " \"input\": \"Documentation for the input parameter\"\n" + " }\n" + " }\n" + "}}"; + + checkNatspec(sourceCode, natspec, false); +} BOOST_AUTO_TEST_SUITE_END() From 69bb2a38b97f25b7973e094a89b9f2126aee8023 Mon Sep 17 00:00:00 2001 From: Lefteris Karapetsas Date: Thu, 4 Dec 2014 18:12:52 +0100 Subject: [PATCH 13/27] Natspec @return tag parsing - Also omitting tags from the output JSON file if they are missing instead of providing an empty string for their value --- libsolidity/InterfaceHandler.cpp | 32 +++++++++++--- libsolidity/InterfaceHandler.h | 4 +- test/solidityNatspecJSON.cpp | 74 +++++++++++++++++++++++++++++++- 3 files changed, 100 insertions(+), 10 deletions(-) diff --git a/libsolidity/InterfaceHandler.cpp b/libsolidity/InterfaceHandler.cpp index 222711e6a..2a6de2c35 100644 --- a/libsolidity/InterfaceHandler.cpp +++ b/libsolidity/InterfaceHandler.cpp @@ -75,8 +75,11 @@ std::unique_ptr InterfaceHandler::getUserDocumentation(std::shared_ { resetUser(); parseDocString(*strPtr); - user["notice"] = Json::Value(m_notice); - methods[f->getName()] = user; + if (!m_notice.empty()) + {// since @notice is the only user tag if missing function should not appear + user["notice"] = Json::Value(m_notice); + methods[f->getName()] = user; + } } } doc["methods"] = methods; @@ -86,6 +89,8 @@ std::unique_ptr InterfaceHandler::getUserDocumentation(std::shared_ std::unique_ptr InterfaceHandler::getDevDocumentation(std::shared_ptr _contractDef) { + // LTODO: Somewhere in this function warnings for mismatch of param names + // should be thrown Json::Value doc; Json::Value methods(Json::objectValue); @@ -98,14 +103,20 @@ std::unique_ptr InterfaceHandler::getDevDocumentation(std::shared_p resetDev(); parseDocString(*strPtr); - method["details"] = Json::Value(m_dev); + if (!m_dev.empty()) + method["details"] = Json::Value(m_dev); Json::Value params(Json::objectValue); for (auto const& pair: m_params) { params[pair.first] = pair.second; } - method["params"] = params; - methods[f->getName()] = method; + if (!m_params.empty()) + method["params"] = params; + if (!m_return.empty()) + method["return"] = m_return; + + if (!method.empty()) // add the function, only if we have any documentation to add + methods[f->getName()] = method; } } doc["methods"] = methods; @@ -122,6 +133,7 @@ void InterfaceHandler::resetUser() void InterfaceHandler::resetDev() { m_dev.clear(); + m_return.clear(); m_params.clear(); } @@ -188,7 +200,7 @@ size_t InterfaceHandler::appendDocTagParam(std::string const& _string, size_t _s size_t InterfaceHandler::parseDocTag(std::string const& _string, std::string const& _tag, size_t _pos) { - //TODO: need to check for @(start of a tag) between here and the end of line + // LTODO: need to check for @(start of a tag) between here and the end of line // for all cases size_t nlPos = _pos; if (m_lastTag == DOCTAG_NONE || _tag != "") @@ -197,11 +209,13 @@ size_t InterfaceHandler::parseDocTag(std::string const& _string, std::string con nlPos = parseDocTagLine(_string, m_dev, _pos, DOCTAG_DEV); else if (_tag == "notice") nlPos = parseDocTagLine(_string, m_notice, _pos, DOCTAG_NOTICE); + else if (_tag == "return") + nlPos = parseDocTagLine(_string, m_return, _pos, DOCTAG_RETURN); else if (_tag == "param") nlPos = parseDocTagParam(_string, _pos); else { - //TODO: Some form of warning + // LTODO: Unknown tas, throw some form of warning } } else @@ -223,6 +237,10 @@ size_t InterfaceHandler::appendDocTag(std::string const& _string, size_t _startP m_notice += " "; newPos = parseDocTagLine(_string, m_notice, _startPos, DOCTAG_NOTICE); break; + case DOCTAG_RETURN: + m_return += " "; + newPos = parseDocTagLine(_string, m_return, _startPos, DOCTAG_RETURN); + break; case DOCTAG_PARAM: newPos = appendDocTagParam(_string, _startPos); break; diff --git a/libsolidity/InterfaceHandler.h b/libsolidity/InterfaceHandler.h index 2a70af950..eeaed033d 100644 --- a/libsolidity/InterfaceHandler.h +++ b/libsolidity/InterfaceHandler.h @@ -42,7 +42,8 @@ enum docTagType DOCTAG_NONE = 0, DOCTAG_DEV, DOCTAG_NOTICE, - DOCTAG_PARAM + DOCTAG_PARAM, + DOCTAG_RETURN }; class InterfaceHandler @@ -91,6 +92,7 @@ private: enum docTagType m_lastTag; std::string m_notice; std::string m_dev; + std::string m_return; std::vector> m_params; }; diff --git a/test/solidityNatspecJSON.cpp b/test/solidityNatspecJSON.cpp index 5894f3b04..2c4be219b 100644 --- a/test/solidityNatspecJSON.cpp +++ b/test/solidityNatspecJSON.cpp @@ -100,8 +100,7 @@ BOOST_AUTO_TEST_CASE(dev_and_user_basic_test) char const* devNatspec = "{" "\"methods\":{" " \"mul\":{ \n" - " \"details\": \"Multiplies a number by 7\",\n" - " \"params\": {}\n" + " \"details\": \"Multiplies a number by 7\"\n" " }\n" " }\n" "}}"; @@ -175,6 +174,24 @@ BOOST_AUTO_TEST_CASE(user_empty_contract) checkNatspec(sourceCode, natspec, true); } +BOOST_AUTO_TEST_CASE(dev_and_user_no_doc) +{ + char const* sourceCode = "contract test {\n" + " function mul(uint a) returns(uint d) { return a * 7; }\n" + " function sub(int input) returns(int d)\n" + " {\n" + " return input - 3;\n" + " }\n" + "}\n"; + + char const* devNatspec = "{\"methods\":{}}"; + + char const* userNatspec = "{\"methods\":{}}"; + + checkNatspec(sourceCode, devNatspec, false); + checkNatspec(sourceCode, userNatspec, true); +} + BOOST_AUTO_TEST_CASE(dev_multiple_params) { char const* sourceCode = "contract test {\n" @@ -272,6 +289,59 @@ BOOST_AUTO_TEST_CASE(dev_multiple_functions) checkNatspec(sourceCode, natspec, false); } +BOOST_AUTO_TEST_CASE(dev_return) +{ + char const* sourceCode = "contract test {\n" + " /// @dev Multiplies a number by 7 and adds second parameter\n" + " /// @param a Documentation for the first parameter starts here.\n" + " /// Since it's a really complicated parameter we need 2 lines\n" + " /// @param second Documentation for the second parameter\n" + " /// @return The result of the multiplication\n" + " function mul(uint a, uint second) returns(uint d) { return a * 7 + second; }\n" + "}\n"; + + char const* natspec = "{" + "\"methods\":{" + " \"mul\":{ \n" + " \"details\": \"Multiplies a number by 7 and adds second parameter\",\n" + " \"params\": {\n" + " \"a\": \"Documentation for the first parameter starts here.Since it's a really complicated parameter we need 2 lines\",\n" + " \"second\": \"Documentation for the second parameter\"\n" + " },\n" + " \"return\": \"The result of the multiplication\"\n" + " }\n" + "}}"; + + checkNatspec(sourceCode, natspec, false); +} + +BOOST_AUTO_TEST_CASE(dev_multiline_return) +{ + char const* sourceCode = "contract test {\n" + " /// @dev Multiplies a number by 7 and adds second parameter\n" + " /// @param a Documentation for the first parameter starts here.\n" + " /// Since it's a really complicated parameter we need 2 lines\n" + " /// @param second Documentation for the second parameter\n" + " /// @return The result of the multiplication\n" + " /// and cookies with nutella\n" + " function mul(uint a, uint second) returns(uint d) { return a * 7 + second; }\n" + "}\n"; + + char const* natspec = "{" + "\"methods\":{" + " \"mul\":{ \n" + " \"details\": \"Multiplies a number by 7 and adds second parameter\",\n" + " \"params\": {\n" + " \"a\": \"Documentation for the first parameter starts here.Since it's a really complicated parameter we need 2 lines\",\n" + " \"second\": \"Documentation for the second parameter\"\n" + " },\n" + " \"return\": \"The result of the multiplication and cookies with nutella\"\n" + " }\n" + "}}"; + + checkNatspec(sourceCode, natspec, false); +} + BOOST_AUTO_TEST_SUITE_END() } From 67da8798cf29b428d06905616b8ef42b1b624a45 Mon Sep 17 00:00:00 2001 From: Lefteris Karapetsas Date: Thu, 4 Dec 2014 23:55:47 +0100 Subject: [PATCH 14/27] Addressing styling and miscellaneous issue with Natspec --- libsolidity/CompilerStack.cpp | 20 ++++++------- libsolidity/CompilerStack.h | 4 +-- libsolidity/InterfaceHandler.cpp | 49 ++++++++++++++------------------ libsolidity/InterfaceHandler.h | 18 ++++++------ 4 files changed, 43 insertions(+), 48 deletions(-) diff --git a/libsolidity/CompilerStack.cpp b/libsolidity/CompilerStack.cpp index 3e9791bd2..3281442fa 100644 --- a/libsolidity/CompilerStack.cpp +++ b/libsolidity/CompilerStack.cpp @@ -36,7 +36,7 @@ namespace dev namespace solidity { -CompilerStack::CompilerStack():m_interfaceHandler(make_shared()){} +CompilerStack::CompilerStack(): m_interfaceHandler(make_shared()) {} void CompilerStack::setSource(string const& _sourceCode) { @@ -84,33 +84,31 @@ void CompilerStack::streamAssembly(ostream& _outStream) m_compiler->streamAssembly(_outStream); } -std::string const* CompilerStack::getJsonDocumentation(enum documentationType _type) +std::string const* CompilerStack::getJsonDocumentation(enum DocumentationType _type) { if (!m_parseSuccessful) BOOST_THROW_EXCEPTION(CompilerError() << errinfo_comment("Parsing was not successful.")); - auto createOrReturnDoc = [this, _type](std::unique_ptr& _doc) + auto createDocIfNotThere = [this, _type](std::unique_ptr& _doc) { - if(!_doc) - { + if (!_doc) _doc = m_interfaceHandler->getDocumentation(m_contractASTNode, _type); - } }; switch (_type) { case NATSPEC_USER: - createOrReturnDoc(m_userDocumentation); + createDocIfNotThere(m_userDocumentation); return m_userDocumentation.get(); case NATSPEC_DEV: - createOrReturnDoc(m_devDocumentation); + createDocIfNotThere(m_devDocumentation); return m_devDocumentation.get(); case ABI_INTERFACE: - createOrReturnDoc(m_interface); + createDocIfNotThere(m_interface); return m_interface.get(); } - BOOST_THROW_EXCEPTION(CompilerError() << errinfo_comment("Internal error")); - return nullptr; + + BOOST_THROW_EXCEPTION(InternalCompilerError() << errinfo_comment("Illegal documentation type.")); } bytes CompilerStack::staticCompile(std::string const& _sourceCode, bool _optimize) diff --git a/libsolidity/CompilerStack.h b/libsolidity/CompilerStack.h index 511951291..de356b1ae 100644 --- a/libsolidity/CompilerStack.h +++ b/libsolidity/CompilerStack.h @@ -37,7 +37,7 @@ class Compiler; class GlobalContext; class InterfaceHandler; -enum documentationType: unsigned short +enum DocumentationType: unsigned short { NATSPEC_USER = 1, NATSPEC_DEV, @@ -74,7 +74,7 @@ public: /// Prerequisite: Successful call to parse or compile. /// @param type The type of the documentation to get. /// Can be one of 3 types defined at @c documentation_type - std::string const* getJsonDocumentation(enum documentationType type); + std::string const* getJsonDocumentation(enum DocumentationType type); /// Returns the previously used scanner, useful for counting lines during error reporting. Scanner const& getScanner() const { return *m_scanner; } diff --git a/libsolidity/InterfaceHandler.cpp b/libsolidity/InterfaceHandler.cpp index 2a6de2c35..53632bc6f 100644 --- a/libsolidity/InterfaceHandler.cpp +++ b/libsolidity/InterfaceHandler.cpp @@ -3,8 +3,10 @@ #include #include -namespace dev { -namespace solidity { +namespace dev +{ +namespace solidity +{ /* -- public -- */ @@ -14,7 +16,7 @@ InterfaceHandler::InterfaceHandler() } std::unique_ptr InterfaceHandler::getDocumentation(std::shared_ptr _contractDef, - enum documentationType _type) + enum DocumentationType _type) { switch(_type) { @@ -34,8 +36,7 @@ std::unique_ptr InterfaceHandler::getABIInterface(std::shared_ptr exportedFunctions = _contractDef->getInterfaceFunctions(); - for (FunctionDefinition const* f: exportedFunctions) + for (FunctionDefinition const* f: _contractDef->getInterfaceFunctions()) { Json::Value method; Json::Value inputs(Json::arrayValue); @@ -107,9 +108,8 @@ std::unique_ptr InterfaceHandler::getDevDocumentation(std::shared_p method["details"] = Json::Value(m_dev); Json::Value params(Json::objectValue); for (auto const& pair: m_params) - { params[pair.first] = pair.second; - } + if (!m_params.empty()) method["params"] = params; if (!m_return.empty()) @@ -145,12 +145,12 @@ size_t skipLineOrEOS(std::string const& _string, size_t _nlPos) size_t InterfaceHandler::parseDocTagLine(std::string const& _string, std::string& _tagString, size_t _pos, - enum docTagType _tagType) + enum DocTagType _tagType) { - size_t nlPos = _string.find("\n", _pos); + size_t nlPos = _string.find('\n', _pos); _tagString += _string.substr(_pos, nlPos == std::string::npos ? - _string.length() : + _string.length() - _pos: nlPos - _pos); m_lastTag = _tagType; return skipLineOrEOS(_string, nlPos); @@ -159,20 +159,18 @@ size_t InterfaceHandler::parseDocTagLine(std::string const& _string, size_t InterfaceHandler::parseDocTagParam(std::string const& _string, size_t _startPos) { // find param name - size_t currPos = _string.find(" ", _startPos); + size_t currPos = _string.find(' ', _startPos); if (currPos == std::string::npos) - { BOOST_THROW_EXCEPTION(CompilerError() << errinfo_comment("End of param name not found")); - return currPos; //no end of tag found - } + auto paramName = _string.substr(_startPos, currPos - _startPos); currPos += 1; - size_t nlPos = _string.find("\n", currPos); + size_t nlPos = _string.find('\n', currPos); auto paramDesc = _string.substr(currPos, nlPos == std::string::npos ? - _string.length() : + _string.length() - currPos : nlPos - currPos); m_params.push_back(std::make_pair(paramName, paramDesc)); @@ -184,13 +182,13 @@ size_t InterfaceHandler::parseDocTagParam(std::string const& _string, size_t _st size_t InterfaceHandler::appendDocTagParam(std::string const& _string, size_t _startPos) { // Should never be called with an empty vector - assert(!m_params.empty()); - + if (asserts(!m_params.empty())) + BOOST_THROW_EXCEPTION(CompilerError() << errinfo_comment("Internal: Tried to append to empty parameter")); auto pair = m_params.back(); - size_t nlPos = _string.find("\n", _startPos); + size_t nlPos = _string.find('\n', _startPos); pair.second += _string.substr(_startPos, nlPos == std::string::npos ? - _string.length() : + _string.length() - _startPos : nlPos - _startPos); m_params.at(m_params.size() - 1) = pair; @@ -227,7 +225,7 @@ size_t InterfaceHandler::parseDocTag(std::string const& _string, std::string con size_t InterfaceHandler::appendDocTag(std::string const& _string, size_t _startPos) { size_t newPos = _startPos; - switch(m_lastTag) + switch (m_lastTag) { case DOCTAG_DEV: m_dev += " "; @@ -254,18 +252,15 @@ void InterfaceHandler::parseDocString(std::string const& _string, size_t _startP { size_t pos2; size_t newPos = _startPos; - size_t tagPos = _string.find("@", _startPos); - size_t nlPos = _string.find("\n", _startPos); + size_t tagPos = _string.find('@', _startPos); + size_t nlPos = _string.find('\n', _startPos); if (tagPos != std::string::npos && tagPos < nlPos) { // we found a tag - pos2 = _string.find(" ", tagPos); + pos2 = _string.find(' ', tagPos); if (pos2 == std::string::npos) - { BOOST_THROW_EXCEPTION(CompilerError() << errinfo_comment("End of tag not found")); - return; //no end of tag found - } newPos = parseDocTag(_string, _string.substr(tagPos + 1, pos2 - tagPos - 1), pos2 + 1); } diff --git a/libsolidity/InterfaceHandler.h b/libsolidity/InterfaceHandler.h index eeaed033d..e9e3a83c8 100644 --- a/libsolidity/InterfaceHandler.h +++ b/libsolidity/InterfaceHandler.h @@ -30,14 +30,16 @@ #include #include -namespace dev { -namespace solidity { +namespace dev +{ +namespace solidity +{ // Forward declarations class ContractDefinition; -enum documentationType: unsigned short; +enum DocumentationType: unsigned short; -enum docTagType +enum DocTagType { DOCTAG_NONE = 0, DOCTAG_DEV, @@ -54,11 +56,11 @@ public: /// Get the given type of documentation /// @param _contractDef The contract definition /// @param _type The type of the documentation. Can be one of the - /// types provided by @c documentation_type + /// types provided by @c DocumentationType /// @return A unique pointer contained string with the json /// representation of provided type std::unique_ptr getDocumentation(std::shared_ptr _contractDef, - enum documentationType _type); + enum DocumentationType _type); /// Get the ABI Interface of the contract /// @param _contractDef The contract definition /// @return A unique pointer contained string with the json @@ -79,7 +81,7 @@ private: void resetUser(); void resetDev(); - size_t parseDocTagLine(std::string const& _string, std::string& _tagString, size_t _pos, enum docTagType _tagType); + size_t parseDocTagLine(std::string const& _string, std::string& _tagString, size_t _pos, enum DocTagType _tagType); size_t parseDocTagParam(std::string const& _string, size_t _startPos); size_t appendDocTagParam(std::string const& _string, size_t _startPos); void parseDocString(std::string const& _string, size_t _startPos = 0); @@ -89,7 +91,7 @@ private: Json::StyledWriter m_writer; // internal state - enum docTagType m_lastTag; + enum DocTagType m_lastTag; std::string m_notice; std::string m_dev; std::string m_return; From 4bbb9eb264f3e9cb9a3947c6eed3a60b0645f281 Mon Sep 17 00:00:00 2001 From: Lefteris Karapetsas Date: Fri, 5 Dec 2014 02:10:54 +0100 Subject: [PATCH 15/27] Using iterators in Natspec comment parsing - Used iterators in the entirety of the InterfaceHandler natspec comment parsing pipeline - Fixed issue where @param continuing in new line would not get a space --- libsolidity/InterfaceHandler.cpp | 138 +++++++++++++++---------------- libsolidity/InterfaceHandler.h | 20 +++-- test/solidityNatspecJSON.cpp | 6 +- 3 files changed, 82 insertions(+), 82 deletions(-) diff --git a/libsolidity/InterfaceHandler.cpp b/libsolidity/InterfaceHandler.cpp index 53632bc6f..1450d8fc3 100644 --- a/libsolidity/InterfaceHandler.cpp +++ b/libsolidity/InterfaceHandler.cpp @@ -137,143 +137,135 @@ void InterfaceHandler::resetDev() m_params.clear(); } -size_t skipLineOrEOS(std::string const& _string, size_t _nlPos) +std::string::const_iterator skipLineOrEOS(std::string::const_iterator _nlPos, + std::string::const_iterator _end) { - return (_nlPos == std::string::npos) ? _string.length() : _nlPos + 1; + return (_nlPos == _end) ? _end : ++_nlPos; } -size_t InterfaceHandler::parseDocTagLine(std::string const& _string, - std::string& _tagString, - size_t _pos, - enum DocTagType _tagType) +std::string::const_iterator InterfaceHandler::parseDocTagLine(std::string::const_iterator _pos, + std::string::const_iterator _end, + std::string& _tagString, + enum DocTagType _tagType) { - size_t nlPos = _string.find('\n', _pos); - _tagString += _string.substr(_pos, - nlPos == std::string::npos ? - _string.length() - _pos: - nlPos - _pos); + auto nlPos = std::find(_pos, _end, '\n'); + std::copy(_pos, nlPos, back_inserter(_tagString)); m_lastTag = _tagType; - return skipLineOrEOS(_string, nlPos); + return skipLineOrEOS(nlPos, _end); } -size_t InterfaceHandler::parseDocTagParam(std::string const& _string, size_t _startPos) +std::string::const_iterator InterfaceHandler::parseDocTagParam(std::string::const_iterator _pos, + std::string::const_iterator _end) { // find param name - size_t currPos = _string.find(' ', _startPos); - if (currPos == std::string::npos) + auto currPos = std::find(_pos, _end, ' '); + if (currPos == _end) BOOST_THROW_EXCEPTION(CompilerError() << errinfo_comment("End of param name not found")); - auto paramName = _string.substr(_startPos, currPos - _startPos); + auto paramName = std::string(_pos, currPos); currPos += 1; - size_t nlPos = _string.find('\n', currPos); - auto paramDesc = _string.substr(currPos, - nlPos == std::string::npos ? - _string.length() - currPos : - nlPos - currPos); - + auto nlPos = std::find(currPos, _end, '\n'); + auto paramDesc = std::string(currPos, nlPos); m_params.push_back(std::make_pair(paramName, paramDesc)); m_lastTag = DOCTAG_PARAM; - return skipLineOrEOS(_string, nlPos); + return skipLineOrEOS(nlPos, _end); } -size_t InterfaceHandler::appendDocTagParam(std::string const& _string, size_t _startPos) +std::string::const_iterator InterfaceHandler::appendDocTagParam(std::string::const_iterator _pos, + std::string::const_iterator _end) { // Should never be called with an empty vector if (asserts(!m_params.empty())) BOOST_THROW_EXCEPTION(CompilerError() << errinfo_comment("Internal: Tried to append to empty parameter")); + auto pair = m_params.back(); - size_t nlPos = _string.find('\n', _startPos); - pair.second += _string.substr(_startPos, - nlPos == std::string::npos ? - _string.length() - _startPos : - nlPos - _startPos); + pair.second += " "; + auto nlPos = std::find(_pos, _end, '\n'); + std::copy(_pos, nlPos, back_inserter(pair.second)); m_params.at(m_params.size() - 1) = pair; - return skipLineOrEOS(_string, nlPos); + return skipLineOrEOS(nlPos, _end); } -size_t InterfaceHandler::parseDocTag(std::string const& _string, std::string const& _tag, size_t _pos) +std::string::const_iterator InterfaceHandler::parseDocTag(std::string::const_iterator _pos, + std::string::const_iterator _end, + std::string const& _tag) { // LTODO: need to check for @(start of a tag) between here and the end of line // for all cases - size_t nlPos = _pos; if (m_lastTag == DOCTAG_NONE || _tag != "") { if (_tag == "dev") - nlPos = parseDocTagLine(_string, m_dev, _pos, DOCTAG_DEV); + return parseDocTagLine(_pos, _end, m_dev, DOCTAG_DEV); else if (_tag == "notice") - nlPos = parseDocTagLine(_string, m_notice, _pos, DOCTAG_NOTICE); + return parseDocTagLine(_pos, _end, m_notice, DOCTAG_NOTICE); else if (_tag == "return") - nlPos = parseDocTagLine(_string, m_return, _pos, DOCTAG_RETURN); + return parseDocTagLine(_pos, _end, m_return, DOCTAG_RETURN); else if (_tag == "param") - nlPos = parseDocTagParam(_string, _pos); + return parseDocTagParam(_pos, _end); else { - // LTODO: Unknown tas, throw some form of warning + // LTODO: Unknown tag, throw some form of warning and not just an exception + BOOST_THROW_EXCEPTION(CompilerError() << errinfo_comment("Unknown tag encountered")); } } else - appendDocTag(_string, _pos); - - return nlPos; + return appendDocTag(_pos, _end); } -size_t InterfaceHandler::appendDocTag(std::string const& _string, size_t _startPos) +std::string::const_iterator InterfaceHandler::appendDocTag(std::string::const_iterator _pos, + std::string::const_iterator _end) { - size_t newPos = _startPos; switch (m_lastTag) { case DOCTAG_DEV: m_dev += " "; - newPos = parseDocTagLine(_string, m_dev, _startPos, DOCTAG_DEV); - break; + return parseDocTagLine(_pos, _end, m_dev, DOCTAG_DEV); case DOCTAG_NOTICE: m_notice += " "; - newPos = parseDocTagLine(_string, m_notice, _startPos, DOCTAG_NOTICE); - break; + return parseDocTagLine(_pos, _end, m_notice, DOCTAG_NOTICE); case DOCTAG_RETURN: m_return += " "; - newPos = parseDocTagLine(_string, m_return, _startPos, DOCTAG_RETURN); - break; + return parseDocTagLine(_pos, _end, m_return, DOCTAG_RETURN); case DOCTAG_PARAM: - newPos = appendDocTagParam(_string, _startPos); - break; + return appendDocTagParam(_pos, _end); default: + BOOST_THROW_EXCEPTION(CompilerError() << errinfo_comment("Internal: Illegal documentation tag")); break; - } - return newPos; + } } -void InterfaceHandler::parseDocString(std::string const& _string, size_t _startPos) +void InterfaceHandler::parseDocString(std::string const& _string) { - size_t pos2; - size_t newPos = _startPos; - size_t tagPos = _string.find('@', _startPos); - size_t nlPos = _string.find('\n', _startPos); + auto currPos = _string.begin(); + auto end = _string.end(); - if (tagPos != std::string::npos && tagPos < nlPos) + while (currPos != end) { - // we found a tag - pos2 = _string.find(' ', tagPos); - if (pos2 == std::string::npos) - BOOST_THROW_EXCEPTION(CompilerError() << errinfo_comment("End of tag not found")); + auto tagPos = std::find(currPos, end, '@'); + auto nlPos = std::find(currPos, end, '\n'); - newPos = parseDocTag(_string, _string.substr(tagPos + 1, pos2 - tagPos - 1), pos2 + 1); - } - else if (m_lastTag != DOCTAG_NONE) // continuation of the previous tag - newPos = appendDocTag(_string, _startPos + 1); - else // skip the line if a newline was found - { - if (newPos != std::string::npos) - newPos = nlPos + 1; + if (tagPos != end && tagPos < nlPos) + { + // we found a tag + auto tagNameEndPos = std::find(tagPos, end, ' '); + if (tagNameEndPos == end) + BOOST_THROW_EXCEPTION(CompilerError() << errinfo_comment("End of tag not found")); + + currPos = parseDocTag(tagNameEndPos + 1, end, std::string(tagPos +1, tagNameEndPos)); + } + else if (m_lastTag != DOCTAG_NONE) // continuation of the previous tag + currPos = appendDocTag(currPos + 1, end); + else // skip the line if a newline was found + { + if (currPos != end) + currPos = nlPos + 1; + } } - if (newPos == std::string::npos || newPos == _string.length()) - return; // EOS - parseDocString(_string, newPos); } } //solidity NS diff --git a/libsolidity/InterfaceHandler.h b/libsolidity/InterfaceHandler.h index e9e3a83c8..7a5ee66db 100644 --- a/libsolidity/InterfaceHandler.h +++ b/libsolidity/InterfaceHandler.h @@ -81,12 +81,20 @@ private: void resetUser(); void resetDev(); - size_t parseDocTagLine(std::string const& _string, std::string& _tagString, size_t _pos, enum DocTagType _tagType); - size_t parseDocTagParam(std::string const& _string, size_t _startPos); - size_t appendDocTagParam(std::string const& _string, size_t _startPos); - void parseDocString(std::string const& _string, size_t _startPos = 0); - size_t appendDocTag(std::string const& _string, size_t _startPos); - size_t parseDocTag(std::string const& _string, std::string const& _tag, size_t _pos); + std::string::const_iterator parseDocTagLine(std::string::const_iterator _pos, + std::string::const_iterator _end, + std::string& _tagString, + enum DocTagType _tagType); + std::string::const_iterator parseDocTagParam(std::string::const_iterator _pos, + std::string::const_iterator _end); + std::string::const_iterator appendDocTagParam(std::string::const_iterator _pos, + std::string::const_iterator _end); + void parseDocString(std::string const& _string); + std::string::const_iterator appendDocTag(std::string::const_iterator _pos, + std::string::const_iterator _end); + std::string::const_iterator parseDocTag(std::string::const_iterator _pos, + std::string::const_iterator _end, + std::string const& _tag); Json::StyledWriter m_writer; diff --git a/test/solidityNatspecJSON.cpp b/test/solidityNatspecJSON.cpp index 2c4be219b..f2ae15c96 100644 --- a/test/solidityNatspecJSON.cpp +++ b/test/solidityNatspecJSON.cpp @@ -230,7 +230,7 @@ BOOST_AUTO_TEST_CASE(dev_mutiline_param_description) " \"mul\":{ \n" " \"details\": \"Multiplies a number by 7 and adds second parameter\",\n" " \"params\": {\n" - " \"a\": \"Documentation for the first parameter starts here.Since it's a really complicated parameter we need 2 lines\",\n" + " \"a\": \"Documentation for the first parameter starts here. Since it's a really complicated parameter we need 2 lines\",\n" " \"second\": \"Documentation for the second parameter\"\n" " }\n" " }\n" @@ -305,7 +305,7 @@ BOOST_AUTO_TEST_CASE(dev_return) " \"mul\":{ \n" " \"details\": \"Multiplies a number by 7 and adds second parameter\",\n" " \"params\": {\n" - " \"a\": \"Documentation for the first parameter starts here.Since it's a really complicated parameter we need 2 lines\",\n" + " \"a\": \"Documentation for the first parameter starts here. Since it's a really complicated parameter we need 2 lines\",\n" " \"second\": \"Documentation for the second parameter\"\n" " },\n" " \"return\": \"The result of the multiplication\"\n" @@ -332,7 +332,7 @@ BOOST_AUTO_TEST_CASE(dev_multiline_return) " \"mul\":{ \n" " \"details\": \"Multiplies a number by 7 and adds second parameter\",\n" " \"params\": {\n" - " \"a\": \"Documentation for the first parameter starts here.Since it's a really complicated parameter we need 2 lines\",\n" + " \"a\": \"Documentation for the first parameter starts here. Since it's a really complicated parameter we need 2 lines\",\n" " \"second\": \"Documentation for the second parameter\"\n" " },\n" " \"return\": \"The result of the multiplication and cookies with nutella\"\n" From cfb8e74a75acac98042a0aef2d26ccfaf6749647 Mon Sep 17 00:00:00 2001 From: Lefteris Karapetsas Date: Fri, 5 Dec 2014 12:08:26 +0100 Subject: [PATCH 16/27] Introducing Docstring parsing error exception and style fixes --- libsolidity/Exceptions.h | 1 + libsolidity/InterfaceHandler.cpp | 22 ++++++++++------------ 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/libsolidity/Exceptions.h b/libsolidity/Exceptions.h index 1903c1dc2..8298c9810 100644 --- a/libsolidity/Exceptions.h +++ b/libsolidity/Exceptions.h @@ -36,6 +36,7 @@ struct TypeError: virtual Exception {}; struct DeclarationError: virtual Exception {}; struct CompilerError: virtual Exception {}; struct InternalCompilerError: virtual Exception {}; +struct DocstringParsingError: virtual Exception {}; typedef boost::error_info errinfo_sourcePosition; typedef boost::error_info errinfo_sourceLocation; diff --git a/libsolidity/InterfaceHandler.cpp b/libsolidity/InterfaceHandler.cpp index 1450d8fc3..ca02cc37d 100644 --- a/libsolidity/InterfaceHandler.cpp +++ b/libsolidity/InterfaceHandler.cpp @@ -28,7 +28,7 @@ std::unique_ptr InterfaceHandler::getDocumentation(std::shared_ptr< return getABIInterface(_contractDef); } - BOOST_THROW_EXCEPTION(CompilerError() << errinfo_comment("Internal error")); + BOOST_THROW_EXCEPTION(InternalCompilerError() << errinfo_comment("Unknown documentation type")); return nullptr; } @@ -160,7 +160,7 @@ std::string::const_iterator InterfaceHandler::parseDocTagParam(std::string::cons // find param name auto currPos = std::find(_pos, _end, ' '); if (currPos == _end) - BOOST_THROW_EXCEPTION(CompilerError() << errinfo_comment("End of param name not found")); + BOOST_THROW_EXCEPTION(DocstringParsingError() << errinfo_comment("End of param name not found" + std::string(_pos, _end))); auto paramName = std::string(_pos, currPos); @@ -179,7 +179,7 @@ std::string::const_iterator InterfaceHandler::appendDocTagParam(std::string::con { // Should never be called with an empty vector if (asserts(!m_params.empty())) - BOOST_THROW_EXCEPTION(CompilerError() << errinfo_comment("Internal: Tried to append to empty parameter")); + BOOST_THROW_EXCEPTION(InternalCompilerError() << errinfo_comment("Internal: Tried to append to empty parameter")); auto pair = m_params.back(); pair.second += " "; @@ -210,7 +210,7 @@ std::string::const_iterator InterfaceHandler::parseDocTag(std::string::const_ite else { // LTODO: Unknown tag, throw some form of warning and not just an exception - BOOST_THROW_EXCEPTION(CompilerError() << errinfo_comment("Unknown tag encountered")); + BOOST_THROW_EXCEPTION(DocstringParsingError() << errinfo_comment("Unknown tag " + _tag + " encountered")); } } else @@ -234,7 +234,7 @@ std::string::const_iterator InterfaceHandler::appendDocTag(std::string::const_it case DOCTAG_PARAM: return appendDocTagParam(_pos, _end); default: - BOOST_THROW_EXCEPTION(CompilerError() << errinfo_comment("Internal: Illegal documentation tag")); + BOOST_THROW_EXCEPTION(InternalCompilerError() << errinfo_comment("Internal: Illegal documentation tag type")); break; } } @@ -254,17 +254,15 @@ void InterfaceHandler::parseDocString(std::string const& _string) // we found a tag auto tagNameEndPos = std::find(tagPos, end, ' '); if (tagNameEndPos == end) - BOOST_THROW_EXCEPTION(CompilerError() << errinfo_comment("End of tag not found")); + BOOST_THROW_EXCEPTION(DocstringParsingError() << + errinfo_comment("End of tag " + std::string(tagPos, tagNameEndPos) + "not found")); - currPos = parseDocTag(tagNameEndPos + 1, end, std::string(tagPos +1, tagNameEndPos)); + currPos = parseDocTag(tagNameEndPos + 1, end, std::string(tagPos + 1, tagNameEndPos)); } else if (m_lastTag != DOCTAG_NONE) // continuation of the previous tag currPos = appendDocTag(currPos + 1, end); - else // skip the line if a newline was found - { - if (currPos != end) - currPos = nlPos + 1; - } + else if (currPos != end) // skip the line if a newline was found + currPos = nlPos + 1; } } From 9977229d759448bf0a6c89a4451bae8852ed02c1 Mon Sep 17 00:00:00 2001 From: Lefteris Karapetsas Date: Fri, 5 Dec 2014 12:27:18 +0100 Subject: [PATCH 17/27] Newline right after doctag is now a valid natspec entry - Plus tests for that --- libsolidity/InterfaceHandler.cpp | 14 +++++++-- test/solidityNatspecJSON.cpp | 51 ++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 3 deletions(-) diff --git a/libsolidity/InterfaceHandler.cpp b/libsolidity/InterfaceHandler.cpp index ca02cc37d..0115c7f59 100644 --- a/libsolidity/InterfaceHandler.cpp +++ b/libsolidity/InterfaceHandler.cpp @@ -137,8 +137,8 @@ void InterfaceHandler::resetDev() m_params.clear(); } -std::string::const_iterator skipLineOrEOS(std::string::const_iterator _nlPos, - std::string::const_iterator _end) +static inline std::string::const_iterator skipLineOrEOS(std::string::const_iterator _nlPos, + std::string::const_iterator _end) { return (_nlPos == _end) ? _end : ++_nlPos; } @@ -239,6 +239,14 @@ std::string::const_iterator InterfaceHandler::appendDocTag(std::string::const_it } } +static inline std::string::const_iterator getFirstSpaceOrNl(std::string::const_iterator _pos, + std::string::const_iterator _end) +{ + auto spacePos = std::find(_pos, _end, ' '); + auto nlPos = std::find(_pos, _end, '\n'); + return (spacePos < nlPos) ? spacePos : nlPos; +} + void InterfaceHandler::parseDocString(std::string const& _string) { auto currPos = _string.begin(); @@ -252,7 +260,7 @@ void InterfaceHandler::parseDocString(std::string const& _string) if (tagPos != end && tagPos < nlPos) { // we found a tag - auto tagNameEndPos = std::find(tagPos, end, ' '); + auto tagNameEndPos = getFirstSpaceOrNl(tagPos, end); if (tagNameEndPos == end) BOOST_THROW_EXCEPTION(DocstringParsingError() << errinfo_comment("End of tag " + std::string(tagPos, tagNameEndPos) + "not found")); diff --git a/test/solidityNatspecJSON.cpp b/test/solidityNatspecJSON.cpp index f2ae15c96..9596f2b80 100644 --- a/test/solidityNatspecJSON.cpp +++ b/test/solidityNatspecJSON.cpp @@ -192,6 +192,30 @@ BOOST_AUTO_TEST_CASE(dev_and_user_no_doc) checkNatspec(sourceCode, userNatspec, true); } +BOOST_AUTO_TEST_CASE(dev_desc_after_nl) +{ + char const* sourceCode = "contract test {\n" + " /// @dev\n" + " /// Multiplies a number by 7 and adds second parameter\n" + " /// @param a Documentation for the first parameter\n" + " /// @param second Documentation for the second parameter\n" + " function mul(uint a, uint second) returns(uint d) { return a * 7 + second; }\n" + "}\n"; + + char const* natspec = "{" + "\"methods\":{" + " \"mul\":{ \n" + " \"details\": \" Multiplies a number by 7 and adds second parameter\",\n" + " \"params\": {\n" + " \"a\": \"Documentation for the first parameter\",\n" + " \"second\": \"Documentation for the second parameter\"\n" + " }\n" + " }\n" + "}}"; + + checkNatspec(sourceCode, natspec, false); +} + BOOST_AUTO_TEST_CASE(dev_multiple_params) { char const* sourceCode = "contract test {\n" @@ -314,6 +338,33 @@ BOOST_AUTO_TEST_CASE(dev_return) checkNatspec(sourceCode, natspec, false); } +BOOST_AUTO_TEST_CASE(dev_return_desc_after_nl) +{ + char const* sourceCode = "contract test {\n" + " /// @dev Multiplies a number by 7 and adds second parameter\n" + " /// @param a Documentation for the first parameter starts here.\n" + " /// Since it's a really complicated parameter we need 2 lines\n" + " /// @param second Documentation for the second parameter\n" + " /// @return\n" + " /// The result of the multiplication\n" + " function mul(uint a, uint second) returns(uint d) { return a * 7 + second; }\n" + "}\n"; + + char const* natspec = "{" + "\"methods\":{" + " \"mul\":{ \n" + " \"details\": \"Multiplies a number by 7 and adds second parameter\",\n" + " \"params\": {\n" + " \"a\": \"Documentation for the first parameter starts here. Since it's a really complicated parameter we need 2 lines\",\n" + " \"second\": \"Documentation for the second parameter\"\n" + " },\n" + " \"return\": \" The result of the multiplication\"\n" + " }\n" + "}}"; + + checkNatspec(sourceCode, natspec, false); +} + BOOST_AUTO_TEST_CASE(dev_multiline_return) { From 78938ac468c4eddedd0c8c66a12094668e224668 Mon Sep 17 00:00:00 2001 From: Lefteris Karapetsas Date: Fri, 5 Dec 2014 12:41:32 +0100 Subject: [PATCH 18/27] Stack compiler now correctly returns a string and not a pointer --- libsolidity/CompilerStack.cpp | 8 ++++---- libsolidity/CompilerStack.h | 2 +- test/solidityJSONInterfaceTest.cpp | 2 +- test/solidityNatspecJSON.cpp | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/libsolidity/CompilerStack.cpp b/libsolidity/CompilerStack.cpp index 3281442fa..c83257a1d 100644 --- a/libsolidity/CompilerStack.cpp +++ b/libsolidity/CompilerStack.cpp @@ -84,7 +84,7 @@ void CompilerStack::streamAssembly(ostream& _outStream) m_compiler->streamAssembly(_outStream); } -std::string const* CompilerStack::getJsonDocumentation(enum DocumentationType _type) +std::string const& CompilerStack::getJsonDocumentation(enum DocumentationType _type) { if (!m_parseSuccessful) BOOST_THROW_EXCEPTION(CompilerError() << errinfo_comment("Parsing was not successful.")); @@ -99,13 +99,13 @@ std::string const* CompilerStack::getJsonDocumentation(enum DocumentationType _t { case NATSPEC_USER: createDocIfNotThere(m_userDocumentation); - return m_userDocumentation.get(); + return *m_userDocumentation; case NATSPEC_DEV: createDocIfNotThere(m_devDocumentation); - return m_devDocumentation.get(); + return *m_devDocumentation; case ABI_INTERFACE: createDocIfNotThere(m_interface); - return m_interface.get(); + return *m_interface; } BOOST_THROW_EXCEPTION(InternalCompilerError() << errinfo_comment("Illegal documentation type.")); diff --git a/libsolidity/CompilerStack.h b/libsolidity/CompilerStack.h index de356b1ae..8dc546fbe 100644 --- a/libsolidity/CompilerStack.h +++ b/libsolidity/CompilerStack.h @@ -74,7 +74,7 @@ public: /// Prerequisite: Successful call to parse or compile. /// @param type The type of the documentation to get. /// Can be one of 3 types defined at @c documentation_type - std::string const* getJsonDocumentation(enum DocumentationType type); + std::string const& getJsonDocumentation(enum DocumentationType type); /// Returns the previously used scanner, useful for counting lines during error reporting. Scanner const& getScanner() const { return *m_scanner; } diff --git a/test/solidityJSONInterfaceTest.cpp b/test/solidityJSONInterfaceTest.cpp index 8fe0ea653..407f05d03 100644 --- a/test/solidityJSONInterfaceTest.cpp +++ b/test/solidityJSONInterfaceTest.cpp @@ -50,7 +50,7 @@ public: msg += *extra; BOOST_FAIL(msg); } - std::string generatedInterfaceString = *m_compilerStack.getJsonDocumentation(ABI_INTERFACE); + std::string generatedInterfaceString = m_compilerStack.getJsonDocumentation(ABI_INTERFACE); Json::Value generatedInterface; m_reader.parse(generatedInterfaceString, generatedInterface); Json::Value expectedInterface; diff --git a/test/solidityNatspecJSON.cpp b/test/solidityNatspecJSON.cpp index 9596f2b80..2ccedf7a2 100644 --- a/test/solidityNatspecJSON.cpp +++ b/test/solidityNatspecJSON.cpp @@ -55,9 +55,9 @@ public: } if (_userDocumentation) - generatedDocumentationString = *m_compilerStack.getJsonDocumentation(NATSPEC_USER); + generatedDocumentationString = m_compilerStack.getJsonDocumentation(NATSPEC_USER); else - generatedDocumentationString = *m_compilerStack.getJsonDocumentation(NATSPEC_DEV); + generatedDocumentationString = m_compilerStack.getJsonDocumentation(NATSPEC_DEV); Json::Value generatedDocumentation; m_reader.parse(generatedDocumentationString, generatedDocumentation); Json::Value expectedDocumentation; From 8857a7a75e8095ef7940deda220a256208e1ba90 Mon Sep 17 00:00:00 2001 From: Lefteris Karapetsas Date: Fri, 5 Dec 2014 15:50:39 +0100 Subject: [PATCH 19/27] Replacing old cstyle enums with c++11 enums in natspec --- libsolidity/CompilerStack.cpp | 8 +++---- libsolidity/CompilerStack.h | 4 ++-- libsolidity/InterfaceHandler.cpp | 38 +++++++++++++++--------------- libsolidity/InterfaceHandler.h | 20 ++++++++-------- solc/main.cpp | 6 ++--- test/solidityJSONInterfaceTest.cpp | 2 +- test/solidityNatspecJSON.cpp | 4 ++-- 7 files changed, 41 insertions(+), 41 deletions(-) diff --git a/libsolidity/CompilerStack.cpp b/libsolidity/CompilerStack.cpp index c83257a1d..01bf99d94 100644 --- a/libsolidity/CompilerStack.cpp +++ b/libsolidity/CompilerStack.cpp @@ -84,7 +84,7 @@ void CompilerStack::streamAssembly(ostream& _outStream) m_compiler->streamAssembly(_outStream); } -std::string const& CompilerStack::getJsonDocumentation(enum DocumentationType _type) +std::string const& CompilerStack::getJsonDocumentation(DocumentationType _type) { if (!m_parseSuccessful) BOOST_THROW_EXCEPTION(CompilerError() << errinfo_comment("Parsing was not successful.")); @@ -97,13 +97,13 @@ std::string const& CompilerStack::getJsonDocumentation(enum DocumentationType _t switch (_type) { - case NATSPEC_USER: + case DocumentationType::NATSPEC_USER: createDocIfNotThere(m_userDocumentation); return *m_userDocumentation; - case NATSPEC_DEV: + case DocumentationType::NATSPEC_DEV: createDocIfNotThere(m_devDocumentation); return *m_devDocumentation; - case ABI_INTERFACE: + case DocumentationType::ABI_INTERFACE: createDocIfNotThere(m_interface); return *m_interface; } diff --git a/libsolidity/CompilerStack.h b/libsolidity/CompilerStack.h index 8dc546fbe..928815cc5 100644 --- a/libsolidity/CompilerStack.h +++ b/libsolidity/CompilerStack.h @@ -37,7 +37,7 @@ class Compiler; class GlobalContext; class InterfaceHandler; -enum DocumentationType: unsigned short +enum class DocumentationType: uint8_t { NATSPEC_USER = 1, NATSPEC_DEV, @@ -74,7 +74,7 @@ public: /// Prerequisite: Successful call to parse or compile. /// @param type The type of the documentation to get. /// Can be one of 3 types defined at @c documentation_type - std::string const& getJsonDocumentation(enum DocumentationType type); + std::string const& getJsonDocumentation(DocumentationType type); /// Returns the previously used scanner, useful for counting lines during error reporting. Scanner const& getScanner() const { return *m_scanner; } diff --git a/libsolidity/InterfaceHandler.cpp b/libsolidity/InterfaceHandler.cpp index 0115c7f59..18c053cb1 100644 --- a/libsolidity/InterfaceHandler.cpp +++ b/libsolidity/InterfaceHandler.cpp @@ -12,19 +12,19 @@ namespace solidity InterfaceHandler::InterfaceHandler() { - m_lastTag = DOCTAG_NONE; + m_lastTag = DocTagType::NONE; } std::unique_ptr InterfaceHandler::getDocumentation(std::shared_ptr _contractDef, - enum DocumentationType _type) + DocumentationType _type) { switch(_type) { - case NATSPEC_USER: + case DocumentationType::NATSPEC_USER: return getUserDocumentation(_contractDef); - case NATSPEC_DEV: + case DocumentationType::NATSPEC_DEV: return getDevDocumentation(_contractDef); - case ABI_INTERFACE: + case DocumentationType::ABI_INTERFACE: return getABIInterface(_contractDef); } @@ -146,7 +146,7 @@ static inline std::string::const_iterator skipLineOrEOS(std::string::const_itera std::string::const_iterator InterfaceHandler::parseDocTagLine(std::string::const_iterator _pos, std::string::const_iterator _end, std::string& _tagString, - enum DocTagType _tagType) + DocTagType _tagType) { auto nlPos = std::find(_pos, _end, '\n'); std::copy(_pos, nlPos, back_inserter(_tagString)); @@ -170,7 +170,7 @@ std::string::const_iterator InterfaceHandler::parseDocTagParam(std::string::cons auto paramDesc = std::string(currPos, nlPos); m_params.push_back(std::make_pair(paramName, paramDesc)); - m_lastTag = DOCTAG_PARAM; + m_lastTag = DocTagType::PARAM; return skipLineOrEOS(nlPos, _end); } @@ -197,14 +197,14 @@ std::string::const_iterator InterfaceHandler::parseDocTag(std::string::const_ite { // LTODO: need to check for @(start of a tag) between here and the end of line // for all cases - if (m_lastTag == DOCTAG_NONE || _tag != "") + if (m_lastTag == DocTagType::NONE || _tag != "") { if (_tag == "dev") - return parseDocTagLine(_pos, _end, m_dev, DOCTAG_DEV); + return parseDocTagLine(_pos, _end, m_dev, DocTagType::DEV); else if (_tag == "notice") - return parseDocTagLine(_pos, _end, m_notice, DOCTAG_NOTICE); + return parseDocTagLine(_pos, _end, m_notice, DocTagType::NOTICE); else if (_tag == "return") - return parseDocTagLine(_pos, _end, m_return, DOCTAG_RETURN); + return parseDocTagLine(_pos, _end, m_return, DocTagType::RETURN); else if (_tag == "param") return parseDocTagParam(_pos, _end); else @@ -222,16 +222,16 @@ std::string::const_iterator InterfaceHandler::appendDocTag(std::string::const_it { switch (m_lastTag) { - case DOCTAG_DEV: + case DocTagType::DEV: m_dev += " "; - return parseDocTagLine(_pos, _end, m_dev, DOCTAG_DEV); - case DOCTAG_NOTICE: + return parseDocTagLine(_pos, _end, m_dev, DocTagType::DEV); + case DocTagType::NOTICE: m_notice += " "; - return parseDocTagLine(_pos, _end, m_notice, DOCTAG_NOTICE); - case DOCTAG_RETURN: + return parseDocTagLine(_pos, _end, m_notice, DocTagType::NOTICE); + case DocTagType::RETURN: m_return += " "; - return parseDocTagLine(_pos, _end, m_return, DOCTAG_RETURN); - case DOCTAG_PARAM: + return parseDocTagLine(_pos, _end, m_return, DocTagType::RETURN); + case DocTagType::PARAM: return appendDocTagParam(_pos, _end); default: BOOST_THROW_EXCEPTION(InternalCompilerError() << errinfo_comment("Internal: Illegal documentation tag type")); @@ -267,7 +267,7 @@ void InterfaceHandler::parseDocString(std::string const& _string) currPos = parseDocTag(tagNameEndPos + 1, end, std::string(tagPos + 1, tagNameEndPos)); } - else if (m_lastTag != DOCTAG_NONE) // continuation of the previous tag + else if (m_lastTag != DocTagType::NONE) // continuation of the previous tag currPos = appendDocTag(currPos + 1, end); else if (currPos != end) // skip the line if a newline was found currPos = nlPos + 1; diff --git a/libsolidity/InterfaceHandler.h b/libsolidity/InterfaceHandler.h index 7a5ee66db..524e2903c 100644 --- a/libsolidity/InterfaceHandler.h +++ b/libsolidity/InterfaceHandler.h @@ -37,15 +37,15 @@ namespace solidity // Forward declarations class ContractDefinition; -enum DocumentationType: unsigned short; +enum class DocumentationType: uint8_t; -enum DocTagType +enum class DocTagType: uint8_t { - DOCTAG_NONE = 0, - DOCTAG_DEV, - DOCTAG_NOTICE, - DOCTAG_PARAM, - DOCTAG_RETURN + NONE = 0, + DEV, + NOTICE, + PARAM, + RETURN }; class InterfaceHandler @@ -60,7 +60,7 @@ public: /// @return A unique pointer contained string with the json /// representation of provided type std::unique_ptr getDocumentation(std::shared_ptr _contractDef, - enum DocumentationType _type); + DocumentationType _type); /// Get the ABI Interface of the contract /// @param _contractDef The contract definition /// @return A unique pointer contained string with the json @@ -84,7 +84,7 @@ private: std::string::const_iterator parseDocTagLine(std::string::const_iterator _pos, std::string::const_iterator _end, std::string& _tagString, - enum DocTagType _tagType); + DocTagType _tagType); std::string::const_iterator parseDocTagParam(std::string::const_iterator _pos, std::string::const_iterator _end); std::string::const_iterator appendDocTagParam(std::string::const_iterator _pos, @@ -99,7 +99,7 @@ private: Json::StyledWriter m_writer; // internal state - enum DocTagType m_lastTag; + DocTagType m_lastTag; std::string m_notice; std::string m_dev; std::string m_return; diff --git a/solc/main.cpp b/solc/main.cpp index daeb2707c..21ffc98cc 100644 --- a/solc/main.cpp +++ b/solc/main.cpp @@ -135,9 +135,9 @@ int main(int argc, char** argv) cout << "Opcodes:" << endl; cout << eth::disassemble(compiler.getBytecode()) << endl; cout << "Binary: " << toHex(compiler.getBytecode()) << endl; - cout << "Interface specification: " << compiler.getJsonDocumentation(ABI_INTERFACE) << endl; - cout << "Natspec user documentation: " << compiler.getJsonDocumentation(NATSPEC_USER) << endl; - cout << "Natspec developer documentation: " << compiler.getJsonDocumentation(NATSPEC_DEV) << endl; + cout << "Interface specification: " << compiler.getJsonDocumentation(DocumentationType::ABI_INTERFACE) << endl; + cout << "Natspec user documentation: " << compiler.getJsonDocumentation(DocumentationType::NATSPEC_USER) << endl; + cout << "Natspec developer documentation: " << compiler.getJsonDocumentation(DocumentationType::NATSPEC_DEV) << endl; return 0; } diff --git a/test/solidityJSONInterfaceTest.cpp b/test/solidityJSONInterfaceTest.cpp index 407f05d03..17e97dc69 100644 --- a/test/solidityJSONInterfaceTest.cpp +++ b/test/solidityJSONInterfaceTest.cpp @@ -50,7 +50,7 @@ public: msg += *extra; BOOST_FAIL(msg); } - std::string generatedInterfaceString = m_compilerStack.getJsonDocumentation(ABI_INTERFACE); + std::string generatedInterfaceString = m_compilerStack.getJsonDocumentation(DocumentationType::ABI_INTERFACE); Json::Value generatedInterface; m_reader.parse(generatedInterfaceString, generatedInterface); Json::Value expectedInterface; diff --git a/test/solidityNatspecJSON.cpp b/test/solidityNatspecJSON.cpp index 2ccedf7a2..5e4c1fca7 100644 --- a/test/solidityNatspecJSON.cpp +++ b/test/solidityNatspecJSON.cpp @@ -55,9 +55,9 @@ public: } if (_userDocumentation) - generatedDocumentationString = m_compilerStack.getJsonDocumentation(NATSPEC_USER); + generatedDocumentationString = m_compilerStack.getJsonDocumentation(DocumentationType::NATSPEC_USER); else - generatedDocumentationString = m_compilerStack.getJsonDocumentation(NATSPEC_DEV); + generatedDocumentationString = m_compilerStack.getJsonDocumentation(DocumentationType::NATSPEC_DEV); Json::Value generatedDocumentation; m_reader.parse(generatedDocumentationString, generatedDocumentation); Json::Value expectedDocumentation; From 4ce6e3483a99f8663deb6a8d4c5ee63063b42b47 Mon Sep 17 00:00:00 2001 From: CJentzsch Date: Thu, 4 Dec 2014 17:55:04 +0100 Subject: [PATCH 20/27] fix stackoverflow in calldataload, codecopy, extcodecopy + some tests --- libevm/VM.h | 40 +++---- test/stPreCompiledContractsFiller.json | 35 ++++++ test/vm.cpp | 20 ++++ test/vmEnvironmentalInfoTestFiller.json | 140 ++++++++++++++++++++++++ 4 files changed, 215 insertions(+), 20 deletions(-) diff --git a/libevm/VM.h b/libevm/VM.h index 5fb46fc68..0e27db02d 100644 --- a/libevm/VM.h +++ b/libevm/VM.h @@ -524,12 +524,12 @@ template dev::bytesConstRef dev::eth::VM::go(Ext& _ext, OnOpFunc con break; case Instruction::CALLDATALOAD: { - if ((unsigned)m_stack.back() + 31 < _ext.data.size()) + if ((unsigned)m_stack.back() + (uint64_t)31 < _ext.data.size()) m_stack.back() = (u256)*(h256 const*)(_ext.data.data() + (unsigned)m_stack.back()); else { h256 r; - for (unsigned i = (unsigned)m_stack.back(), e = (unsigned)m_stack.back() + 32, j = 0; i < e; ++i, ++j) + for (uint64_t i = (unsigned)m_stack.back(), e = (unsigned)m_stack.back() + 32, j = 0; i < e; ++i, ++j) r[j] = i < _ext.data.size() ? _ext.data[i] : 0; m_stack.back() = (u256)r; } @@ -540,15 +540,15 @@ template dev::bytesConstRef dev::eth::VM::go(Ext& _ext, OnOpFunc con break; case Instruction::CALLDATACOPY: { - unsigned mf = (unsigned)m_stack.back(); + unsigned offset = (unsigned)m_stack.back(); m_stack.pop_back(); - u256 cf = m_stack.back(); + u256 dataIndex = m_stack.back(); m_stack.pop_back(); - unsigned l = (unsigned)m_stack.back(); + unsigned size = (unsigned)m_stack.back(); m_stack.pop_back(); - unsigned el = cf + l > (u256)_ext.data.size() ? (u256)_ext.data.size() < cf ? 0 : _ext.data.size() - (unsigned)cf : l; - memcpy(m_temp.data() + mf, _ext.data.data() + (unsigned)cf, el); - memset(m_temp.data() + mf + el, 0, l - el); + unsigned el = dataIndex + (bigint)size > (u256)_ext.data.size() ? (u256)_ext.data.size() < dataIndex ? 0 : _ext.data.size() - (unsigned)dataIndex : size; + memcpy(m_temp.data() + offset, _ext.data.data() + (unsigned)dataIndex, el); + memset(m_temp.data() + offset + el, 0, size - el); break; } case Instruction::CODESIZE: @@ -556,15 +556,15 @@ template dev::bytesConstRef dev::eth::VM::go(Ext& _ext, OnOpFunc con break; case Instruction::CODECOPY: { - unsigned mf = (unsigned)m_stack.back(); + unsigned offset = (unsigned)m_stack.back(); m_stack.pop_back(); - u256 cf = (u256)m_stack.back(); + u256 dataIndex = (u256)m_stack.back(); m_stack.pop_back(); - unsigned l = (unsigned)m_stack.back(); + unsigned size = (unsigned)m_stack.back(); m_stack.pop_back(); - unsigned el = cf + l > (u256)_ext.code.size() ? (u256)_ext.code.size() < cf ? 0 : _ext.code.size() - (unsigned)cf : l; - memcpy(m_temp.data() + mf, _ext.code.data() + (unsigned)cf, el); - memset(m_temp.data() + mf + el, 0, l - el); + unsigned el = dataIndex + (bigint)size > (u256)_ext.code.size() ? (u256)_ext.code.size() < dataIndex ? 0 : _ext.code.size() - (unsigned)dataIndex : size; + memcpy(m_temp.data() + offset, _ext.code.data() + (unsigned)dataIndex, el); + memset(m_temp.data() + offset + el, 0, size - el); break; } case Instruction::EXTCODESIZE: @@ -574,15 +574,15 @@ template dev::bytesConstRef dev::eth::VM::go(Ext& _ext, OnOpFunc con { Address a = asAddress(m_stack.back()); m_stack.pop_back(); - unsigned mf = (unsigned)m_stack.back(); + unsigned offset = (unsigned)m_stack.back(); m_stack.pop_back(); - u256 cf = m_stack.back(); + u256 dataIndex = m_stack.back(); m_stack.pop_back(); - unsigned l = (unsigned)m_stack.back(); + unsigned size = (unsigned)m_stack.back(); m_stack.pop_back(); - unsigned el = cf + l > (u256)_ext.codeAt(a).size() ? (u256)_ext.codeAt(a).size() < cf ? 0 : _ext.codeAt(a).size() - (unsigned)cf : l; - memcpy(m_temp.data() + mf, _ext.codeAt(a).data() + (unsigned)cf, el); - memset(m_temp.data() + mf + el, 0, l - el); + unsigned el = dataIndex + (bigint)size > (u256)_ext.codeAt(a).size() ? (u256)_ext.codeAt(a).size() < dataIndex ? 0 : _ext.codeAt(a).size() - (unsigned)dataIndex : size; + memcpy(m_temp.data() + offset, _ext.codeAt(a).data() + (unsigned)dataIndex, el); + memset(m_temp.data() + offset + el, 0, size - el); break; } case Instruction::GASPRICE: diff --git a/test/stPreCompiledContractsFiller.json b/test/stPreCompiledContractsFiller.json index 9c65ad37b..62a3a1623 100644 --- a/test/stPreCompiledContractsFiller.json +++ b/test/stPreCompiledContractsFiller.json @@ -33,6 +33,41 @@ } }, + "CallEcrecover0_overlappingInputOutput": { + "env" : { + "previousHash" : "5e20a0453cecd065ea59c37ac63e079ee08998b6045136a8ce6635c7912ec0b6", + "currentNumber" : "0", + "currentGasLimit" : "10000000", + "currentDifficulty" : "256", + "currentTimestamp" : 1, + "currentCoinbase" : "2adc25665018aa1fe0e6bc666dac8fc2697ff9ba" + }, + "pre" : { + "095e7baea6a6c7c4c2dfeb977efac326af552d87" : { + "balance" : "20000000", + "nonce" : 0, + "code": "{ (MSTORE 0 0x18c547e4f7b0f325ad1e56f57e26c745b09a3e503d86e00e5255ff7f715d3d1c) (MSTORE 32 28) (MSTORE 64 0x73b1693892219d736caba55bdb67216e485557ea6b6af75f37096c9aa6a5a75f) (MSTORE 96 0xeeb940b1d03b21e36b0e47e79769f095fe2ab855bd91e3a38756b7d75a9c4549) [[ 2 ]] (CALL 1000 1 0 0 128 64 32) [[ 0 ]] (MOD (MLOAD 64) (EXP 2 160)) [[ 1 ]] (EQ (ORIGIN) (SLOAD 0)) }", + "storage": {} + }, + "a94f5374fce5edbc8e2a8697c15331677e6ebf0b" : { + "balance" : "1000000000000000000", + "nonce" : 0, + "code" : "", + "storage": {} + } + }, + "transaction" : { + "nonce" : "0", + "gasPrice" : "1", + "gasLimit" : "365224", + "to" : "095e7baea6a6c7c4c2dfeb977efac326af552d87", + "value" : "100000", + "secretKey" : "45a915e4d060149eb4365960e6a7a45f334393093061116b197e3240065ff2d8", + "data" : "" + } + }, + + "CallEcrecover0_completeReturnValue": { "env" : { "previousHash" : "5e20a0453cecd065ea59c37ac63e079ee08998b6045136a8ce6635c7912ec0b6", diff --git a/test/vm.cpp b/test/vm.cpp index d7bc0612a..e674d6de3 100644 --- a/test/vm.cpp +++ b/test/vm.cpp @@ -488,6 +488,26 @@ BOOST_AUTO_TEST_CASE(vmLogTest) dev::test::executeTests("vmLogTest", "/VMTests", dev::test::doVMTests); } +BOOST_AUTO_TEST_CASE(vmPerformanceTest) +{ + for (int i = 1; i < boost::unit_test::framework::master_test_suite().argc; ++i) + { + string arg = boost::unit_test::framework::master_test_suite().argv[i]; + if (arg == "--performance") + dev::test::executeTests("vmPerformanceTest", "/VMTests", dev::test::doVMTests); + } +} + +BOOST_AUTO_TEST_CASE(vmArithPerformanceTest) +{ + for (int i = 1; i < boost::unit_test::framework::master_test_suite().argc; ++i) + { + string arg = boost::unit_test::framework::master_test_suite().argv[i]; + if (arg == "--performance") + dev::test::executeTests("vmArithPerformanceTest", "/VMTests", dev::test::doVMTests); + } +} + BOOST_AUTO_TEST_CASE(vmRandom) { string testPath = getTestPath(); diff --git a/test/vmEnvironmentalInfoTestFiller.json b/test/vmEnvironmentalInfoTestFiller.json index 95e7936aa..abeed9945 100644 --- a/test/vmEnvironmentalInfoTestFiller.json +++ b/test/vmEnvironmentalInfoTestFiller.json @@ -338,6 +338,33 @@ } }, + "calldataloadSizeTooHigh": { + "env" : { + "previousHash" : "5e20a0453cecd065ea59c37ac63e079ee08998b6045136a8ce6635c7912ec0b6", + "currentNumber" : "0", + "currentGasLimit" : "1000000", + "currentDifficulty" : "256", + "currentTimestamp" : 1, + "currentCoinbase" : "2adc25665018aa1fe0e6bc666dac8fc2697ff9ba" + }, + "pre" : { + "0f572e5295c57f15886f9b263e2f6d2d6c7b5ec6" : { + "balance" : "1000000000000000000", + "nonce" : 0, + "code" : "{ [[ 0 ]] (CALLDATALOAD 0xfffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffa)}", + "storage": {} + } + }, + "exec" : { + "address" : "0f572e5295c57f15886f9b263e2f6d2d6c7b5ec6", + "origin" : "cd1722f3947def4cf144679da39c4c32bdc35681", + "caller" : "cd1722f3947def4cf144679da39c4c32bdc35681", + "value" : "1000000000000000000", + "data" : "0x123456789abcdef0000000000000000000000000000000000000000000000000024", + "gasPrice" : "1000000000", + "gas" : "100000000000" + } + }, "calldatasize0": { "env" : { @@ -451,6 +478,62 @@ } }, + "calldatacopy_DataIndexTooHigh": { + "env" : { + "previousHash" : "5e20a0453cecd065ea59c37ac63e079ee08998b6045136a8ce6635c7912ec0b6", + "currentNumber" : "0", + "currentGasLimit" : "1000000", + "currentDifficulty" : "256", + "currentTimestamp" : 1, + "currentCoinbase" : "2adc25665018aa1fe0e6bc666dac8fc2697ff9ba" + }, + "pre" : { + "0f572e5295c57f15886f9b263e2f6d2d6c7b5ec6" : { + "balance" : "1000000000000000000", + "nonce" : 0, + "code" : "{ (CALLDATACOPY 0 0xfffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffa 0xff ) [[ 0 ]] @0}", + "storage": {} + } + }, + "exec" : { + "address" : "0f572e5295c57f15886f9b263e2f6d2d6c7b5ec6", + "origin" : "cd1722f3947def4cf144679da39c4c32bdc35681", + "caller" : "cd1722f3947def4cf144679da39c4c32bdc35681", + "value" : "1000000000000000000", + "data" : "0x1234567890abcdef01234567890abcdef", + "gasPrice" : "1000000000", + "gas" : "100000000000" + } + }, + + "calldatacopy_DataIndexTooHigh2": { + "env" : { + "previousHash" : "5e20a0453cecd065ea59c37ac63e079ee08998b6045136a8ce6635c7912ec0b6", + "currentNumber" : "0", + "currentGasLimit" : "1000000", + "currentDifficulty" : "256", + "currentTimestamp" : 1, + "currentCoinbase" : "2adc25665018aa1fe0e6bc666dac8fc2697ff9ba" + }, + "pre" : { + "0f572e5295c57f15886f9b263e2f6d2d6c7b5ec6" : { + "balance" : "1000000000000000000", + "nonce" : 0, + "code" : "{ (CALLDATACOPY 0 0xfffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffa 9 ) [[ 0 ]] @0}", + "storage": {} + } + }, + "exec" : { + "address" : "0f572e5295c57f15886f9b263e2f6d2d6c7b5ec6", + "origin" : "cd1722f3947def4cf144679da39c4c32bdc35681", + "caller" : "cd1722f3947def4cf144679da39c4c32bdc35681", + "value" : "1000000000000000000", + "data" : "0x1234567890abcdef01234567890abcdef", + "gasPrice" : "1000000000", + "gas" : "100000000000" + } + }, + "calldatacopy1": { "env" : { "previousHash" : "5e20a0453cecd065ea59c37ac63e079ee08998b6045136a8ce6635c7912ec0b6", @@ -535,6 +618,34 @@ } }, + "codecopy_DataIndexTooHigh": { + "env" : { + "previousHash" : "5e20a0453cecd065ea59c37ac63e079ee08998b6045136a8ce6635c7912ec0b6", + "currentNumber" : "0", + "currentGasLimit" : "1000000", + "currentDifficulty" : "256", + "currentTimestamp" : 1, + "currentCoinbase" : "2adc25665018aa1fe0e6bc666dac8fc2697ff9ba" + }, + "pre" : { + "0f572e5295c57f15886f9b263e2f6d2d6c7b5ec6" : { + "balance" : "1000000000000000000", + "nonce" : 0, + "code" : "{ (CODECOPY 0 0xfffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffa 8 ) [[ 0 ]] @0}", + "storage": {} + } + }, + "exec" : { + "address" : "0f572e5295c57f15886f9b263e2f6d2d6c7b5ec6", + "origin" : "cd1722f3947def4cf144679da39c4c32bdc35681", + "caller" : "cd1722f3947def4cf144679da39c4c32bdc35681", + "value" : "1000000000000000000", + "data" : "0x1234567890abcdef01234567890abcdef", + "gasPrice" : "1000000000", + "gas" : "100000000000" + } + }, + "codecopy0": { "env" : { "previousHash" : "5e20a0453cecd065ea59c37ac63e079ee08998b6045136a8ce6635c7912ec0b6", @@ -686,6 +797,35 @@ } }, + "extcodecopy_DataIndexTooHigh": { + "env" : { + "previousHash" : "5e20a0453cecd065ea59c37ac63e079ee08998b6045136a8ce6635c7912ec0b6", + "currentNumber" : "0", + "currentGasLimit" : "1000000", + "currentDifficulty" : "256", + "currentTimestamp" : 1, + "currentCoinbase" : "2adc25665018aa1fe0e6bc666dac8fc2697ff9ba" + }, + "pre" : { + "0f572e5295c57f15886f9b263e2f6d2d6c7b5ec6" : { + "balance" : "1000000000000000000", + "nonce" : 0, + "code" : "{ (EXTCODECOPY (ADDRESS) 0 0xfffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffa 8 ) [[ 0 ]] @0}", + "storage": {} + } + }, + "exec" : { + "address" : "0f572e5295c57f15886f9b263e2f6d2d6c7b5ec6", + "origin" : "cd1722f3947def4cf144679da39c4c32bdc35681", + "caller" : "cd1722f3947def4cf144679da39c4c32bdc35681", + "value" : "1000000000000000000", + "data" : "0x1234567890abcdef01234567890abcdef", + "gasPrice" : "1000000000", + "gas" : "100000000000" + } + }, + + "extcodecopy0": { "env" : { "previousHash" : "5e20a0453cecd065ea59c37ac63e079ee08998b6045136a8ce6635c7912ec0b6", From 5d6fcb963d57b0f4f7ae86ea80fbb94043506e4d Mon Sep 17 00:00:00 2001 From: CJentzsch Date: Thu, 4 Dec 2014 18:17:03 +0100 Subject: [PATCH 21/27] avoid code repetition in vm --- libevm/VM.h | 58 +++++++++++++++++++++++++---------------------------- 1 file changed, 27 insertions(+), 31 deletions(-) diff --git a/libevm/VM.h b/libevm/VM.h index 0e27db02d..d846ff079 100644 --- a/libevm/VM.h +++ b/libevm/VM.h @@ -538,50 +538,46 @@ template dev::bytesConstRef dev::eth::VM::go(Ext& _ext, OnOpFunc con case Instruction::CALLDATASIZE: m_stack.push_back(_ext.data.size()); break; - case Instruction::CALLDATACOPY: - { - unsigned offset = (unsigned)m_stack.back(); - m_stack.pop_back(); - u256 dataIndex = m_stack.back(); - m_stack.pop_back(); - unsigned size = (unsigned)m_stack.back(); - m_stack.pop_back(); - unsigned el = dataIndex + (bigint)size > (u256)_ext.data.size() ? (u256)_ext.data.size() < dataIndex ? 0 : _ext.data.size() - (unsigned)dataIndex : size; - memcpy(m_temp.data() + offset, _ext.data.data() + (unsigned)dataIndex, el); - memset(m_temp.data() + offset + el, 0, size - el); - break; - } case Instruction::CODESIZE: m_stack.push_back(_ext.code.size()); break; - case Instruction::CODECOPY: - { - unsigned offset = (unsigned)m_stack.back(); - m_stack.pop_back(); - u256 dataIndex = (u256)m_stack.back(); - m_stack.pop_back(); - unsigned size = (unsigned)m_stack.back(); - m_stack.pop_back(); - unsigned el = dataIndex + (bigint)size > (u256)_ext.code.size() ? (u256)_ext.code.size() < dataIndex ? 0 : _ext.code.size() - (unsigned)dataIndex : size; - memcpy(m_temp.data() + offset, _ext.code.data() + (unsigned)dataIndex, el); - memset(m_temp.data() + offset + el, 0, size - el); - break; - } case Instruction::EXTCODESIZE: m_stack.back() = _ext.codeAt(asAddress(m_stack.back())).size(); break; + case Instruction::CALLDATACOPY: + case Instruction::CODECOPY: case Instruction::EXTCODECOPY: { - Address a = asAddress(m_stack.back()); - m_stack.pop_back(); + Address a; + if (inst == Instruction::EXTCODECOPY) + { + a = asAddress(m_stack.back()); + m_stack.pop_back(); + } unsigned offset = (unsigned)m_stack.back(); m_stack.pop_back(); - u256 dataIndex = m_stack.back(); + u256 index = m_stack.back(); m_stack.pop_back(); unsigned size = (unsigned)m_stack.back(); m_stack.pop_back(); - unsigned el = dataIndex + (bigint)size > (u256)_ext.codeAt(a).size() ? (u256)_ext.codeAt(a).size() < dataIndex ? 0 : _ext.codeAt(a).size() - (unsigned)dataIndex : size; - memcpy(m_temp.data() + offset, _ext.codeAt(a).data() + (unsigned)dataIndex, el); + unsigned el; + switch(inst) + { + case Instruction::CALLDATACOPY: + el = index + (bigint)size > (u256)_ext.data.size() ? (u256)_ext.data.size() < index ? 0 : _ext.data.size() - (unsigned)index : size; + memcpy(m_temp.data() + offset, _ext.data.data() + (unsigned)index, el); + break; + case Instruction::CODECOPY: + el = index + (bigint)size > (u256)_ext.code.size() ? (u256)_ext.code.size() < index ? 0 : _ext.code.size() - (unsigned)index : size; + memcpy(m_temp.data() + offset, _ext.code.data() + (unsigned)index, el); + break; + case Instruction::EXTCODECOPY: + el = index + (bigint)size > (u256)_ext.codeAt(a).size() ? (u256)_ext.codeAt(a).size() < index ? 0 : _ext.codeAt(a).size() - (unsigned)index : size; + memcpy(m_temp.data() + offset, _ext.codeAt(a).data() + (unsigned)index, el); + break; + default: + break; + } memset(m_temp.data() + offset + el, 0, size - el); break; } From 0f52c325629abb76498e75e8b880d57d0da8325d Mon Sep 17 00:00:00 2001 From: CJentzsch Date: Thu, 4 Dec 2014 19:23:48 +0100 Subject: [PATCH 22/27] minor fix --- libevm/VM.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libevm/VM.h b/libevm/VM.h index d846ff079..f5feead40 100644 --- a/libevm/VM.h +++ b/libevm/VM.h @@ -529,7 +529,7 @@ template dev::bytesConstRef dev::eth::VM::go(Ext& _ext, OnOpFunc con else { h256 r; - for (uint64_t i = (unsigned)m_stack.back(), e = (unsigned)m_stack.back() + 32, j = 0; i < e; ++i, ++j) + for (uint64_t i = (unsigned)m_stack.back(), e = (unsigned)m_stack.back() + (uint64_t)32, j = 0; i < e; ++i, ++j) r[j] = i < _ext.data.size() ? _ext.data[i] : 0; m_stack.back() = (u256)r; } From 028060c36f87e1cdd75a868090fcab7505b6e049 Mon Sep 17 00:00:00 2001 From: CJentzsch Date: Fri, 5 Dec 2014 11:34:30 +0100 Subject: [PATCH 23/27] even less code --- libevm/VM.h | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/libevm/VM.h b/libevm/VM.h index f5feead40..be64c0ad1 100644 --- a/libevm/VM.h +++ b/libevm/VM.h @@ -560,24 +560,23 @@ template dev::bytesConstRef dev::eth::VM::go(Ext& _ext, OnOpFunc con m_stack.pop_back(); unsigned size = (unsigned)m_stack.back(); m_stack.pop_back(); - unsigned el; + bytes toBeCopied; switch(inst) { case Instruction::CALLDATACOPY: - el = index + (bigint)size > (u256)_ext.data.size() ? (u256)_ext.data.size() < index ? 0 : _ext.data.size() - (unsigned)index : size; - memcpy(m_temp.data() + offset, _ext.data.data() + (unsigned)index, el); + toBeCopied = _ext.data.toBytes(); break; case Instruction::CODECOPY: - el = index + (bigint)size > (u256)_ext.code.size() ? (u256)_ext.code.size() < index ? 0 : _ext.code.size() - (unsigned)index : size; - memcpy(m_temp.data() + offset, _ext.code.data() + (unsigned)index, el); + toBeCopied = _ext.code; break; case Instruction::EXTCODECOPY: - el = index + (bigint)size > (u256)_ext.codeAt(a).size() ? (u256)_ext.codeAt(a).size() < index ? 0 : _ext.codeAt(a).size() - (unsigned)index : size; - memcpy(m_temp.data() + offset, _ext.codeAt(a).data() + (unsigned)index, el); + toBeCopied = _ext.codeAt(a); break; default: break; } + unsigned el = index + (bigint)size > (u256)toBeCopied.size() ? (u256)toBeCopied.size() < index ? 0 : toBeCopied.size() - (unsigned)index : size; + memcpy(m_temp.data() + offset, toBeCopied.data() + (unsigned)index, el); memset(m_temp.data() + offset + el, 0, size - el); break; } From 70ad8ec508860c13a6f2e6a8bb6dba91f0e910b0 Mon Sep 17 00:00:00 2001 From: CJentzsch Date: Fri, 5 Dec 2014 12:30:14 +0100 Subject: [PATCH 24/27] Add exception in vm --- libevm/VM.h | 1 + 1 file changed, 1 insertion(+) diff --git a/libevm/VM.h b/libevm/VM.h index be64c0ad1..3808455a9 100644 --- a/libevm/VM.h +++ b/libevm/VM.h @@ -573,6 +573,7 @@ template dev::bytesConstRef dev::eth::VM::go(Ext& _ext, OnOpFunc con toBeCopied = _ext.codeAt(a); break; default: + BOOST_THROW_EXCEPTION(InvalidOpcode() << errinfo_comment("CALLDATACOPY, CODECOPY or EXTCODECOPY instruction requested.")); break; } unsigned el = index + (bigint)size > (u256)toBeCopied.size() ? (u256)toBeCopied.size() < index ? 0 : toBeCopied.size() - (unsigned)index : size; From 0b2be9812180a004654f7e55648c9afeafd893cf Mon Sep 17 00:00:00 2001 From: CJentzsch Date: Fri, 5 Dec 2014 20:14:42 +0100 Subject: [PATCH 25/27] avoid copy in vm --- libevm/VM.h | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/libevm/VM.h b/libevm/VM.h index 3808455a9..2885deb3b 100644 --- a/libevm/VM.h +++ b/libevm/VM.h @@ -560,25 +560,27 @@ template dev::bytesConstRef dev::eth::VM::go(Ext& _ext, OnOpFunc con m_stack.pop_back(); unsigned size = (unsigned)m_stack.back(); m_stack.pop_back(); - bytes toBeCopied; + unsigned sizeToBeCopied; switch(inst) { case Instruction::CALLDATACOPY: - toBeCopied = _ext.data.toBytes(); + sizeToBeCopied = index + (bigint)size > (u256)_ext.data.size() ? (u256)_ext.data.size() < index ? 0 : _ext.data.size() - (unsigned)index : size; + memcpy(m_temp.data() + offset, _ext.data.data() + (unsigned)index, sizeToBeCopied); break; case Instruction::CODECOPY: - toBeCopied = _ext.code; + sizeToBeCopied = index + (bigint)size > (u256)_ext.code.size() ? (u256)_ext.code.size() < index ? 0 : _ext.code.size() - (unsigned)index : size; + memcpy(m_temp.data() + offset, _ext.code.data() + (unsigned)index, sizeToBeCopied); break; case Instruction::EXTCODECOPY: - toBeCopied = _ext.codeAt(a); + sizeToBeCopied = index + (bigint)size > (u256)_ext.codeAt(a).size() ? (u256)_ext.codeAt(a).size() < index ? 0 : _ext.codeAt(a).size() - (unsigned)index : size; + memcpy(m_temp.data() + offset, _ext.codeAt(a).data() + (unsigned)index, sizeToBeCopied); break; default: + // this is unreachable, but if someone introduces a bug in the future, he may get here. BOOST_THROW_EXCEPTION(InvalidOpcode() << errinfo_comment("CALLDATACOPY, CODECOPY or EXTCODECOPY instruction requested.")); break; } - unsigned el = index + (bigint)size > (u256)toBeCopied.size() ? (u256)toBeCopied.size() < index ? 0 : toBeCopied.size() - (unsigned)index : size; - memcpy(m_temp.data() + offset, toBeCopied.data() + (unsigned)index, el); - memset(m_temp.data() + offset + el, 0, size - el); + memset(m_temp.data() + offset + sizeToBeCopied, 0, size - sizeToBeCopied); break; } case Instruction::GASPRICE: From a23251e9ace072b5ec98838a8d37ca6c90a20880 Mon Sep 17 00:00:00 2001 From: subtly Date: Sat, 6 Dec 2014 01:40:03 +0100 Subject: [PATCH 26/27] network: move static system-network functions into Network class. Further simplifaction of network lifecycle. --- libp2p/Host.cpp | 484 +++++++++++++-------------------------- libp2p/Host.h | 43 ++-- libp2p/Network.cpp | 187 +++++++++++++++ libp2p/Network.h | 63 +++++ libp2p/Session.cpp | 6 + libwebthree/WebThree.cpp | 4 +- libwebthree/WebThree.h | 5 +- 7 files changed, 433 insertions(+), 359 deletions(-) create mode 100644 libp2p/Network.cpp create mode 100644 libp2p/Network.h diff --git a/libp2p/Host.cpp b/libp2p/Host.cpp index 7d08910aa..04c9c8a85 100644 --- a/libp2p/Host.cpp +++ b/libp2p/Host.cpp @@ -15,22 +15,11 @@ along with cpp-ethereum. If not, see . */ /** @file Host.cpp - * @authors: - * Gav Wood - * Eric Lombrozo (Windows version of populateAddresses()) + * @author Alex Leverington + * @author Gav Wood * @date 2014 */ -#include "Host.h" - -#include -#ifdef _WIN32 -// winsock is already included -// #include -#else -#include -#endif - #include #include #include @@ -43,166 +32,18 @@ #include "Common.h" #include "Capability.h" #include "UPnP.h" +#include "Host.h" using namespace std; using namespace dev; using namespace dev::p2p; -std::vector Host::getInterfaceAddresses() -{ - std::vector addresses; - -#ifdef _WIN32 - WSAData wsaData; - if (WSAStartup(MAKEWORD(1, 1), &wsaData) != 0) - BOOST_THROW_EXCEPTION(NoNetworking()); - - char ac[80]; - if (gethostname(ac, sizeof(ac)) == SOCKET_ERROR) - { - clog(NetWarn) << "Error " << WSAGetLastError() << " when getting local host name."; - WSACleanup(); - BOOST_THROW_EXCEPTION(NoNetworking()); - } - - struct hostent* phe = gethostbyname(ac); - if (phe == 0) - { - clog(NetWarn) << "Bad host lookup."; - WSACleanup(); - BOOST_THROW_EXCEPTION(NoNetworking()); - } - - for (int i = 0; phe->h_addr_list[i] != 0; ++i) - { - struct in_addr addr; - memcpy(&addr, phe->h_addr_list[i], sizeof(struct in_addr)); - char *addrStr = inet_ntoa(addr); - bi::address address(bi::address::from_string(addrStr)); - if (!isLocalHostAddress(address)) - addresses.push_back(address.to_v4()); - } - - WSACleanup(); -#else - ifaddrs* ifaddr; - if (getifaddrs(&ifaddr) == -1) - BOOST_THROW_EXCEPTION(NoNetworking()); - - for (auto ifa = ifaddr; ifa != NULL; ifa = ifa->ifa_next) - { - if (!ifa->ifa_addr || string(ifa->ifa_name) == "lo0") - continue; - - if (ifa->ifa_addr->sa_family == AF_INET) - { - in_addr addr = ((struct sockaddr_in *)ifa->ifa_addr)->sin_addr; - boost::asio::ip::address_v4 address(boost::asio::detail::socket_ops::network_to_host_long(addr.s_addr)); - if (!isLocalHostAddress(address)) - addresses.push_back(address); - } - else if (ifa->ifa_addr->sa_family == AF_INET6) - { - sockaddr_in6* sockaddr = ((struct sockaddr_in6 *)ifa->ifa_addr); - in6_addr addr = sockaddr->sin6_addr; - boost::asio::ip::address_v6::bytes_type bytes; - memcpy(&bytes[0], addr.s6_addr, 16); - boost::asio::ip::address_v6 address(bytes, sockaddr->sin6_scope_id); - if (!isLocalHostAddress(address)) - addresses.push_back(address); - } - } - - if (ifaddr!=NULL) - freeifaddrs(ifaddr); - -#endif - - return std::move(addresses); -} - -int Host::listen4(bi::tcp::acceptor* _acceptor, unsigned short _listenPort) -{ - int retport = -1; - for (unsigned i = 0; i < 2; ++i) - { - // try to connect w/listenPort, else attempt net-allocated port - bi::tcp::endpoint endpoint(bi::tcp::v4(), i ? 0 : _listenPort); - try - { - _acceptor->open(endpoint.protocol()); - _acceptor->set_option(ba::socket_base::reuse_address(true)); - _acceptor->bind(endpoint); - _acceptor->listen(); - retport = _acceptor->local_endpoint().port(); - break; - } - catch (...) - { - if (i) - { - // both attempts failed - cwarn << "Couldn't start accepting connections on host. Something very wrong with network?\n" << boost::current_exception_diagnostic_information(); - } - - // first attempt failed - _acceptor->close(); - continue; - } - } - return retport; -} - -bi::tcp::endpoint Host::traverseNAT(std::vector const& _ifAddresses, unsigned short _listenPort, bi::address& o_upnpifaddr) -{ - asserts(_listenPort != 0); - - UPnP* upnp = nullptr; - try - { - upnp = new UPnP; - } - // let m_upnp continue as null - we handle it properly. - catch (NoUPnPDevice) {} - - bi::tcp::endpoint upnpep; - if (upnp && upnp->isValid()) - { - bi::address paddr; - int extPort = 0; - for (auto const& addr: _ifAddresses) - if (addr.is_v4() && isPrivateAddress(addr) && (extPort = upnp->addRedirect(addr.to_string().c_str(), _listenPort))) - { - paddr = addr; - break; - } - - auto eip = upnp->externalIP(); - bi::address eipaddr(bi::address::from_string(eip)); - if (extPort && eip != string("0.0.0.0") && !isPrivateAddress(eipaddr)) - { - clog(NetNote) << "Punched through NAT and mapped local port" << _listenPort << "onto external port" << extPort << "."; - clog(NetNote) << "External addr:" << eip; - o_upnpifaddr = paddr; - upnpep = bi::tcp::endpoint(eipaddr, (unsigned short)extPort); - } - else - clog(NetWarn) << "Couldn't punch through NAT (or no NAT in place)."; - - if (upnp) - delete upnp; - } - - return upnpep; -} - Host::Host(std::string const& _clientVersion, NetworkPreferences const& _n, bool _start): Worker("p2p", 0), m_clientVersion(_clientVersion), m_netPrefs(_n), - m_ifAddresses(getInterfaceAddresses()), - m_ioService(new ba::io_service(2)), - m_acceptor(new bi::tcp::acceptor(*m_ioService)), - m_socket(new bi::tcp::socket(*m_ioService)), + m_ifAddresses(Network::getInterfaceAddresses()), + m_ioService(2), + m_acceptorV4(m_ioService), m_key(KeyPair::create()) { for (auto address: m_ifAddresses) @@ -216,7 +57,7 @@ Host::Host(std::string const& _clientVersion, NetworkPreferences const& _n, bool Host::~Host() { - quit(); + stop(); } void Host::start() @@ -226,30 +67,78 @@ void Host::start() void Host::stop() { + // called to force io_service to kill any remaining tasks it might have - + // such tasks may involve socket reads from Capabilities that maintain references + // to resources we're about to free. + { - // prevent m_run from being set to false at same time as set to true by start() + // Although m_run is set by stop() or start(), it effects m_runTimer so x_runTimer is used instead of a mutex for m_run. + // when m_run == false, run() will cause this::run() to stop() ioservice Guard l(x_runTimer); - // once m_run is false the scheduler will shutdown network and stopWorking() + // ignore if already stopped/stopping + if (!m_run) + return; m_run = false; } - // we know shutdown is complete when m_timer is reset - while (m_timer) + // wait for m_timer to reset (indicating network scheduler has stopped) + while (!!m_timer) this_thread::sleep_for(chrono::milliseconds(50)); + + // stop worker thread stopWorking(); } -void Host::quit() +void Host::doneWorking() { - // called to force io_service to kill any remaining tasks it might have - - // such tasks may involve socket reads from Capabilities that maintain references - // to resources we're about to free. - if (isWorking()) - stop(); - m_acceptor.reset(); - m_socket.reset(); + // reset ioservice (allows manually polling network, below) + m_ioService.reset(); + + // shutdown acceptor + m_acceptorV4.cancel(); + if (m_acceptorV4.is_open()) + m_acceptorV4.close(); + + // There maybe an incoming connection which started but hasn't finished. + // Wait for acceptor to end itself instead of assuming it's complete. + // This helps ensure a peer isn't stopped at the same time it's starting + // and that socket for pending connection is closed. + while (m_accepting) + m_ioService.poll(); + + // stop capabilities (eth: stops syncing or block/tx broadcast) + for (auto const& h: m_capabilities) + h.second->onStopping(); + + // disconnect peers + for (unsigned n = 0;; n = 0) + { + { + RecursiveGuard l(x_peers); + for (auto i: m_peers) + if (auto p = i.second.lock()) + if (p->isOpen()) + { + p->disconnect(ClientQuit); + n++; + } + } + if (!n) + break; + + // poll so that peers send out disconnect packets + m_ioService.poll(); + } + + // stop network (again; helpful to call before subsequent reset()) + m_ioService.stop(); + + // reset network (allows reusing ioservice in future) m_ioService.reset(); - // m_acceptor & m_socket are DANGEROUS now. + + // finally, clear out peers (in case they're lingering) + RecursiveGuard l(x_peers); + m_peers.clear(); } unsigned Host::protocolVersion() const @@ -407,7 +296,7 @@ void Host::determinePublic(string const& _publicAddress, bool _upnp) if (_upnp) { bi::address upnpifaddr; - bi::tcp::endpoint upnpep = traverseNAT(m_ifAddresses, m_listenPort, upnpifaddr); + bi::tcp::endpoint upnpep = Network::traverseNAT(m_ifAddresses, m_listenPort, upnpifaddr); if (!upnpep.address().is_unspecified() && !upnpifaddr.is_unspecified()) { if (!m_peerAddresses.count(upnpep.address())) @@ -431,18 +320,18 @@ void Host::determinePublic(string const& _publicAddress, bool _upnp) m_public = bi::tcp::endpoint(bi::address(), m_listenPort); } -void Host::ensureAccepting() +void Host::runAcceptor() { - // return if there's no io-server (quit called) or we're not listening - if (!m_ioService || m_listenPort < 1) - return; - - if (!m_accepting) + assert(m_listenPort > 0); + + if (m_run && !m_accepting) { clog(NetConnect) << "Listening on local port " << m_listenPort << " (public: " << m_public << ")"; m_accepting = true; - m_acceptor->async_accept(*m_socket, [=](boost::system::error_code ec) + m_socket.reset(new bi::tcp::socket(m_ioService)); + m_acceptorV4.async_accept(*m_socket, [=](boost::system::error_code ec) { + bool success = false; if (!ec) { try @@ -452,8 +341,9 @@ void Host::ensureAccepting() } catch (...){} bi::address remoteAddress = m_socket->remote_endpoint().address(); // Port defaults to 0 - we let the hello tell us which port the peer listens to - auto p = std::make_shared(this, std::move(*m_socket), bi::tcp::endpoint(remoteAddress, 0)); + auto p = std::make_shared(this, std::move(*m_socket.release()), bi::tcp::endpoint(remoteAddress, 0)); p->start(); + success = true; } catch (Exception const& _e) { @@ -464,9 +354,18 @@ void Host::ensureAccepting() clog(NetWarn) << "ERROR: " << _e.what(); } } + + if (!success) + if (m_socket->is_open()) + { + boost::system::error_code ec; + m_socket->shutdown(boost::asio::ip::tcp::socket::shutdown_both, ec); + m_socket->close(); + } + m_accepting = false; if (ec.value() < 1) - ensureAccepting(); + runAcceptor(); }); } } @@ -480,17 +379,16 @@ string Host::pocHost() void Host::connect(std::string const& _addr, unsigned short _port) noexcept { - // if there's no ioService, it means we've had quit() called - bomb out - we're not allowed in here. - if (!m_ioService) + if (!m_run) return; - for (int i = 0; i < 2; ++i) + for (auto first: {true, false}) { try { - if (i == 0) + if (first) { - bi::tcp::resolver r(*m_ioService); + bi::tcp::resolver r(m_ioService); connect(r.resolve({_addr, toString(_port)})->endpoint()); } else @@ -512,12 +410,11 @@ void Host::connect(std::string const& _addr, unsigned short _port) noexcept void Host::connect(bi::tcp::endpoint const& _ep) { - // if there's no ioService, it means we've had quit() called - bomb out - we're not allowed in here. - if (!m_ioService) + if (!m_run) return; clog(NetConnect) << "Attempting single-shot connection to " << _ep; - bi::tcp::socket* s = new bi::tcp::socket(*m_ioService); + bi::tcp::socket* s = new bi::tcp::socket(m_ioService); s->async_connect(_ep, [=](boost::system::error_code const& ec) { if (ec) @@ -534,8 +431,7 @@ void Host::connect(bi::tcp::endpoint const& _ep) void Host::connect(std::shared_ptr const& _n) { - // if there's no ioService, it means we've had quit() called - bomb out - we're not allowed in here. - if (!m_ioService) + if (!m_run) return; // prevent concurrently connecting to a node; todo: better abstraction @@ -551,7 +447,7 @@ void Host::connect(std::shared_ptr const& _n) _n->lastAttempted = std::chrono::system_clock::now(); _n->failedAttempts++; m_ready -= _n->index; - bi::tcp::socket* s = new bi::tcp::socket(*m_ioService); + bi::tcp::socket* s = new bi::tcp::socket(m_ioService); s->async_connect(_n->address, [=](boost::system::error_code const& ec) { if (ec) @@ -680,8 +576,7 @@ void Host::prunePeers() PeerInfos Host::peers(bool _updatePing) const { - // if there's no ioService, it means we've had quit() called - bomb out - we're not allowed in here. - if (!m_ioService) + if (!m_run) return PeerInfos(); RecursiveGuard l(x_peers); @@ -698,152 +593,95 @@ PeerInfos Host::peers(bool _updatePing) const return ret; } -void Host::run(boost::system::error_code const& error) +void Host::run(boost::system::error_code const&) { - m_lastTick += c_timerInterval; - - if (error || !m_ioService) + if (!m_run) { - // timer died or io service went away, so stop here + // stopping io service allows running manual network operations for shutdown + // and also stops blocking worker thread, allowing worker thread to exit + m_ioService.stop(); + + // resetting timer signals network that nothing else can be scheduled to run m_timer.reset(); return; } - // network running - if (m_run) + m_lastTick += c_timerInterval; + if (m_lastTick >= c_timerInterval * 10) { - if (m_lastTick >= c_timerInterval * 10) - { - growPeers(); - prunePeers(); - m_lastTick = 0; - } - - if (m_hadNewNodes) - { - for (auto p: m_peers) - if (auto pp = p.second.lock()) - pp->serviceNodesRequest(); - - m_hadNewNodes = false; - } - - if (chrono::steady_clock::now() - m_lastPing > chrono::seconds(30)) // ping every 30s. - { - for (auto p: m_peers) - if (auto pp = p.second.lock()) - if (chrono::steady_clock::now() - pp->m_lastReceived > chrono::seconds(60)) - pp->disconnect(PingTimeout); - pingAll(); - } - - auto runcb = [this](boost::system::error_code const& error) -> void { run(error); }; - m_timer->expires_from_now(boost::posix_time::milliseconds(c_timerInterval)); - m_timer->async_wait(runcb); - - return; + growPeers(); + prunePeers(); + m_lastTick = 0; } - // network stopping - if (!m_run) + if (m_hadNewNodes) { - // close acceptor - if (m_acceptor->is_open()) - { - if (m_accepting) - m_acceptor->cancel(); - m_acceptor->close(); - m_accepting = false; - } - - // stop capabilities (eth: stops syncing or block/tx broadcast) - for (auto const& h: m_capabilities) - h.second->onStopping(); + for (auto p: m_peers) + if (auto pp = p.second.lock()) + pp->serviceNodesRequest(); - // disconnect peers - for (unsigned n = 0;; n = 0) - { - { - RecursiveGuard l(x_peers); - for (auto i: m_peers) - if (auto p = i.second.lock()) - if (p->isOpen()) - { - p->disconnect(ClientQuit); - n++; - } - } - if (!n) - break; - this_thread::sleep_for(chrono::milliseconds(100)); - } - - if (m_socket->is_open()) - m_socket->close(); - - // m_run is false, so we're stopping; kill timer - m_lastTick = 0; - - // causes parent thread's stop() to continue which calls stopWorking() - m_timer.reset(); - - // stop ioservice (stops blocking worker thread, allowing thread to join) - if (!!m_ioService) - m_ioService->stop(); - return; + m_hadNewNodes = false; } + + if (chrono::steady_clock::now() - m_lastPing > chrono::seconds(30)) // ping every 30s. + { + for (auto p: m_peers) + if (auto pp = p.second.lock()) + if (chrono::steady_clock::now() - pp->m_lastReceived > chrono::seconds(60)) + pp->disconnect(PingTimeout); + pingAll(); + } + + auto runcb = [this](boost::system::error_code const& error) -> void { run(error); }; + m_timer->expires_from_now(boost::posix_time::milliseconds(c_timerInterval)); + m_timer->async_wait(runcb); } void Host::startedWorking() { - if (!m_timer) + asserts(!m_timer); + { - // no timer means this is first run and network must be started - // (run once when host worker thread calls startedWorking()) - - { - // prevent m_run from being set to true at same time as set to false by stop() - // don't release mutex until m_timer is set so in case stop() is called at same - // time, stop will wait on m_timer and graceful network shutdown. - Guard l(x_runTimer); - // reset io service and create deadline timer - m_timer.reset(new boost::asio::deadline_timer(*m_ioService)); - m_run = true; - } - m_ioService->reset(); - - // try to open acceptor (todo: ipv6) - m_listenPort = listen4(m_acceptor.get(), m_netPrefs.listenPort); - - // start capability threads - for (auto const& h: m_capabilities) - h.second->onStarting(); - - // determine public IP, but only if we're able to listen for connections - // todo: GUI when listen is unavailable in UI - if (m_listenPort) - { - determinePublic(m_netPrefs.publicIP, m_netPrefs.upnp); - ensureAccepting(); - } - - // if m_public address is valid then add us to node list - // todo: abstract empty() and emplace logic - if (!m_public.address().is_unspecified() && (m_nodes.empty() || m_nodes[m_nodesList[0]]->id != id())) - noteNode(id(), m_public, Origin::Perfect, false); + // prevent m_run from being set to true at same time as set to false by stop() + // don't release mutex until m_timer is set so in case stop() is called at same + // time, stop will wait on m_timer and graceful network shutdown. + Guard l(x_runTimer); + // create deadline timer + m_timer.reset(new boost::asio::deadline_timer(m_ioService)); + m_run = true; + } + + // try to open acceptor (todo: ipv6) + m_listenPort = Network::listen4(m_acceptorV4, m_netPrefs.listenPort); + + // start capability threads + for (auto const& h: m_capabilities) + h.second->onStarting(); + + // determine public IP, but only if we're able to listen for connections + // todo: GUI when listen is unavailable in UI + if (m_listenPort) + { + determinePublic(m_netPrefs.publicIP, m_netPrefs.upnp); - clog(NetNote) << "Id:" << id().abridged(); + if (m_listenPort > 0) + runAcceptor(); } + // if m_public address is valid then add us to node list + // todo: abstract empty() and emplace logic + if (!m_public.address().is_unspecified() && (m_nodes.empty() || m_nodes[m_nodesList[0]]->id != id())) + noteNode(id(), m_public, Origin::Perfect, false); + + clog(NetNote) << "Id:" << id().abridged(); + run(boost::system::error_code()); } void Host::doWork() { - // no ioService means we've had quit() called - bomb out - we're not allowed in here. - if (asserts(!!m_ioService)) - return; - m_ioService->run(); + if (m_run) + m_ioService.run(); } void Host::pingAll() diff --git a/libp2p/Host.h b/libp2p/Host.h index 644afeb69..bc0e83174 100644 --- a/libp2p/Host.h +++ b/libp2p/Host.h @@ -14,7 +14,8 @@ You should have received a copy of the GNU General Public License along with cpp-ethereum. If not, see . */ -/** @file EthereumHost.h +/** @file Host.h + * @author Alex Leverington * @author Gav Wood * @date 2014 */ @@ -34,9 +35,10 @@ #include #include #include "HostCapability.h" +#include "Network.h" #include "Common.h" namespace ba = boost::asio; -namespace bi = boost::asio::ip; +namespace bi = ba::ip; namespace dev { @@ -101,16 +103,6 @@ struct Node using Nodes = std::vector; -struct NetworkPreferences -{ - NetworkPreferences(unsigned short p = 30303, std::string i = std::string(), bool u = true, bool l = false): listenPort(p), publicIP(i), upnp(u), localNetworking(l) {} - - unsigned short listenPort = 30303; - std::string publicIP; - bool upnp = true; - bool localNetworking = false; -}; - /** * @brief The Host class * Capabilities should be registered prior to startNetwork, since m_capabilities is not thread-safe. @@ -120,17 +112,6 @@ class Host: public Worker friend class Session; friend class HostCapabilityFace; friend struct Node; - - /// Static network interface methods -public: - /// @returns public and private interface addresses - static std::vector getInterfaceAddresses(); - - /// Try to bind and listen on _listenPort, else attempt net-allocated port. - static int listen4(bi::tcp::acceptor* _acceptor, unsigned short _listenPort); - - /// Return public endpoint of upnp interface. If successful o_upnpifaddr will be a private interface address and endpoint will contain public address and port. - static bi::tcp::endpoint traverseNAT(std::vector const& _ifAddresses, unsigned short _listenPort, bi::address& o_upnpifaddr); public: /// Start server, listening for connections on the given port. @@ -187,14 +168,12 @@ public: void start(); /// Stop network. @threadsafe + /// Resets acceptor, socket, and IO service. Called by deallocator. void stop(); /// @returns if network is running. bool isStarted() const { return m_run; } - /// Reset acceptor, socket, and IO service. Called by deallocator. Maybe called by implementation when ordered deallocation is required. - void quit(); - NodeId id() const { return m_key.pub(); } void registerPeer(std::shared_ptr _s, CapDescs const& _caps); @@ -205,7 +184,8 @@ private: /// Populate m_peerAddresses with available public addresses. void determinePublic(std::string const& _publicAddress, bool _upnp); - void ensureAccepting(); + /// Called only from startedWorking(). + void runAcceptor(); void seal(bytes& _b); @@ -217,8 +197,11 @@ private: /// Called by startedWorking. Not thread-safe; to be called only be worker callback. void run(boost::system::error_code const& error); ///< Run network. Called serially via ASIO deadline timer. Manages connection state transitions. - /// Run network + /// Run network. Called by Worker. Not thread-safe; to be called only by worker. virtual void doWork(); + + /// Shutdown network. Called by Worker. Not thread-safe; to be called only by worker. + virtual void doneWorking(); std::shared_ptr noteNode(NodeId _id, bi::tcp::endpoint _a, Origin _o, bool _ready, NodeId _oldId = NodeId()); Nodes potentialPeers(RangeMask const& _known); @@ -235,8 +218,8 @@ private: int m_listenPort = -1; ///< What port are we listening on. -1 means binding failed or acceptor hasn't been initialized. - std::unique_ptr m_ioService; ///< IOService for network stuff. - std::unique_ptr m_acceptor; ///< Listening acceptor. + ba::io_service m_ioService; ///< IOService for network stuff. + bi::tcp::acceptor m_acceptorV4; ///< Listening acceptor. std::unique_ptr m_socket; ///< Listening socket. std::unique_ptr m_timer; ///< Timer which, when network is running, calls scheduler() every c_timerInterval ms. diff --git a/libp2p/Network.cpp b/libp2p/Network.cpp new file mode 100644 index 000000000..8ca8dd135 --- /dev/null +++ b/libp2p/Network.cpp @@ -0,0 +1,187 @@ +/* + This file is part of cpp-ethereum. + + cpp-ethereum is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + cpp-ethereum is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with cpp-ethereum. If not, see . +*/ +/** @file Network.cpp + * @author Alex Leverington + * @author Gav Wood + * @author Eric Lombrozo (Windows version of getInterfaceAddresses()) + * @date 2014 + */ + +#include +#ifndef _WIN32 +#include +#endif + +#include +#include +#include +#include +#include "Common.h" +#include "UPnP.h" +#include "Network.h" + +using namespace std; +using namespace dev; +using namespace dev::p2p; + +std::vector Network::getInterfaceAddresses() +{ + std::vector addresses; + +#ifdef _WIN32 + WSAData wsaData; + if (WSAStartup(MAKEWORD(1, 1), &wsaData) != 0) + BOOST_THROW_EXCEPTION(NoNetworking()); + + char ac[80]; + if (gethostname(ac, sizeof(ac)) == SOCKET_ERROR) + { + clog(NetWarn) << "Error " << WSAGetLastError() << " when getting local host name."; + WSACleanup(); + BOOST_THROW_EXCEPTION(NoNetworking()); + } + + struct hostent* phe = gethostbyname(ac); + if (phe == 0) + { + clog(NetWarn) << "Bad host lookup."; + WSACleanup(); + BOOST_THROW_EXCEPTION(NoNetworking()); + } + + for (int i = 0; phe->h_addr_list[i] != 0; ++i) + { + struct in_addr addr; + memcpy(&addr, phe->h_addr_list[i], sizeof(struct in_addr)); + char *addrStr = inet_ntoa(addr); + bi::address address(bi::address::from_string(addrStr)); + if (!isLocalHostAddress(address)) + addresses.push_back(address.to_v4()); + } + + WSACleanup(); +#else + ifaddrs* ifaddr; + if (getifaddrs(&ifaddr) == -1) + BOOST_THROW_EXCEPTION(NoNetworking()); + + for (auto ifa = ifaddr; ifa != NULL; ifa = ifa->ifa_next) + { + if (!ifa->ifa_addr || string(ifa->ifa_name) == "lo0") + continue; + + if (ifa->ifa_addr->sa_family == AF_INET) + { + in_addr addr = ((struct sockaddr_in *)ifa->ifa_addr)->sin_addr; + boost::asio::ip::address_v4 address(boost::asio::detail::socket_ops::network_to_host_long(addr.s_addr)); + if (!isLocalHostAddress(address)) + addresses.push_back(address); + } + else if (ifa->ifa_addr->sa_family == AF_INET6) + { + sockaddr_in6* sockaddr = ((struct sockaddr_in6 *)ifa->ifa_addr); + in6_addr addr = sockaddr->sin6_addr; + boost::asio::ip::address_v6::bytes_type bytes; + memcpy(&bytes[0], addr.s6_addr, 16); + boost::asio::ip::address_v6 address(bytes, sockaddr->sin6_scope_id); + if (!isLocalHostAddress(address)) + addresses.push_back(address); + } + } + + if (ifaddr!=NULL) + freeifaddrs(ifaddr); + +#endif + + return std::move(addresses); +} + +int Network::listen4(bi::tcp::acceptor& _acceptor, unsigned short _listenPort) +{ + int retport = -1; + for (unsigned i = 0; i < 2; ++i) + { + // try to connect w/listenPort, else attempt net-allocated port + bi::tcp::endpoint endpoint(bi::tcp::v4(), i ? 0 : _listenPort); + try + { + _acceptor.open(endpoint.protocol()); + _acceptor.set_option(ba::socket_base::reuse_address(true)); + _acceptor.bind(endpoint); + _acceptor.listen(); + retport = _acceptor.local_endpoint().port(); + break; + } + catch (...) + { + if (i) + { + // both attempts failed + cwarn << "Couldn't start accepting connections on host. Something very wrong with network?\n" << boost::current_exception_diagnostic_information(); + } + + // first attempt failed + _acceptor.close(); + continue; + } + } + return retport; +} + +bi::tcp::endpoint Network::traverseNAT(std::vector const& _ifAddresses, unsigned short _listenPort, bi::address& o_upnpifaddr) +{ + asserts(_listenPort != 0); + + UPnP* upnp = nullptr; + try + { + upnp = new UPnP; + } + // let m_upnp continue as null - we handle it properly. + catch (NoUPnPDevice) {} + + bi::tcp::endpoint upnpep; + if (upnp && upnp->isValid()) + { + bi::address paddr; + int extPort = 0; + for (auto const& addr: _ifAddresses) + if (addr.is_v4() && isPrivateAddress(addr) && (extPort = upnp->addRedirect(addr.to_string().c_str(), _listenPort))) + { + paddr = addr; + break; + } + + auto eip = upnp->externalIP(); + bi::address eipaddr(bi::address::from_string(eip)); + if (extPort && eip != string("0.0.0.0") && !isPrivateAddress(eipaddr)) + { + clog(NetNote) << "Punched through NAT and mapped local port" << _listenPort << "onto external port" << extPort << "."; + clog(NetNote) << "External addr:" << eip; + o_upnpifaddr = paddr; + upnpep = bi::tcp::endpoint(eipaddr, (unsigned short)extPort); + } + else + clog(NetWarn) << "Couldn't punch through NAT (or no NAT in place)."; + + if (upnp) + delete upnp; + } + + return upnpep; +} diff --git a/libp2p/Network.h b/libp2p/Network.h new file mode 100644 index 000000000..944d390c8 --- /dev/null +++ b/libp2p/Network.h @@ -0,0 +1,63 @@ +/* + This file is part of cpp-ethereum. + + cpp-ethereum is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + cpp-ethereum is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with cpp-ethereum. If not, see . +*/ +/** @file Network.h + * @author Alex Leverington + * @author Gav Wood + * @date 2014 + */ + +#pragma once + +#include +namespace ba = boost::asio; +namespace bi = ba::ip; + +namespace dev +{ + +namespace p2p +{ + +struct NetworkPreferences +{ + NetworkPreferences(unsigned short p = 30303, std::string i = std::string(), bool u = true, bool l = false): listenPort(p), publicIP(i), upnp(u), localNetworking(l) {} + + unsigned short listenPort = 30303; + std::string publicIP; + bool upnp = true; + bool localNetworking = false; +}; + +/** + * @brief Network Class + * Static network operations and interface(s). + */ +class Network +{ +public: + /// @returns public and private interface addresses + static std::vector getInterfaceAddresses(); + + /// Try to bind and listen on _listenPort, else attempt net-allocated port. + static int listen4(bi::tcp::acceptor& _acceptor, unsigned short _listenPort); + + /// Return public endpoint of upnp interface. If successful o_upnpifaddr will be a private interface address and endpoint will contain public address and port. + static bi::tcp::endpoint traverseNAT(std::vector const& _ifAddresses, unsigned short _listenPort, bi::address& o_upnpifaddr); +}; + +} +} diff --git a/libp2p/Session.cpp b/libp2p/Session.cpp index 958473870..cb0a60a92 100644 --- a/libp2p/Session.cpp +++ b/libp2p/Session.cpp @@ -75,7 +75,11 @@ Session::~Session() try { if (m_socket.is_open()) + { + boost::system::error_code ec; + m_socket.shutdown(boost::asio::ip::tcp::socket::shutdown_both, ec); m_socket.close(); + } } catch (...){} } @@ -479,6 +483,8 @@ void Session::drop(DisconnectReason _reason) try { clogS(NetConnect) << "Closing " << m_socket.remote_endpoint() << "(" << reasonOf(_reason) << ")"; + boost::system::error_code ec; + m_socket.shutdown(boost::asio::ip::tcp::socket::shutdown_both, ec); m_socket.close(); } catch (...) {} diff --git a/libwebthree/WebThree.cpp b/libwebthree/WebThree.cpp index b5256ac22..c9f9d56e3 100644 --- a/libwebthree/WebThree.cpp +++ b/libwebthree/WebThree.cpp @@ -57,11 +57,11 @@ WebThreeDirect::~WebThreeDirect() // eth::Client (owned by us via a unique_ptr) uses eth::EthereumHost (via a weak_ptr). // Really need to work out a clean way of organising ownership and guaranteeing startup/shutdown is perfect. - // Have to call quit here to get the Host to kill its io_service otherwise we end up with left-over reads, + // Have to call stop here to get the Host to kill its io_service otherwise we end up with left-over reads, // still referencing Sessions getting deleted *after* m_ethereum is reset, causing bad things to happen, since // the guarantee is that m_ethereum is only reset *after* all sessions have ended (sessions are allowed to // use bits of data owned by m_ethereum). - m_net.quit(); + m_net.stop(); m_ethereum.reset(); } diff --git a/libwebthree/WebThree.h b/libwebthree/WebThree.h index a135f77ff..ec7bf2406 100644 --- a/libwebthree/WebThree.h +++ b/libwebthree/WebThree.h @@ -14,7 +14,7 @@ You should have received a copy of the GNU General Public License along with cpp-ethereum. If not, see . */ -/** @file Client.h +/** @file WebThree.h * @author Gav Wood * @date 2014 */ @@ -92,9 +92,6 @@ public: /// Connect to a particular peer. void connect(std::string const& _seedHost, unsigned short _port = 30303); - /// Is the network subsystem up? - bool haveNetwork() { return peerCount() != 0; } - /// Save peers dev::bytes saveNodes(); From ce92f4dc7235f8c402c293e54a2920dd3b753f9e Mon Sep 17 00:00:00 2001 From: subtly Date: Sat, 6 Dec 2014 02:18:14 +0100 Subject: [PATCH 27/27] cleanup --- libp2p/Host.cpp | 13 ++++++------- libp2p/Host.h | 6 +++--- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/libp2p/Host.cpp b/libp2p/Host.cpp index 04c9c8a85..15521cb28 100644 --- a/libp2p/Host.cpp +++ b/libp2p/Host.cpp @@ -355,13 +355,12 @@ void Host::runAcceptor() } } - if (!success) - if (m_socket->is_open()) - { - boost::system::error_code ec; - m_socket->shutdown(boost::asio::ip::tcp::socket::shutdown_both, ec); - m_socket->close(); - } + if (!success && m_socket->is_open()) + { + boost::system::error_code ec; + m_socket->shutdown(boost::asio::ip::tcp::socket::shutdown_both, ec); + m_socket->close(); + } m_accepting = false; if (ec.value() < 1) diff --git a/libp2p/Host.h b/libp2p/Host.h index bc0e83174..a146d6a66 100644 --- a/libp2p/Host.h +++ b/libp2p/Host.h @@ -194,13 +194,13 @@ private: /// Called by Worker. Not thread-safe; to be called only by worker. virtual void startedWorking(); - /// Called by startedWorking. Not thread-safe; to be called only be worker callback. + /// Called by startedWorking. Not thread-safe; to be called only be Worker. void run(boost::system::error_code const& error); ///< Run network. Called serially via ASIO deadline timer. Manages connection state transitions. - /// Run network. Called by Worker. Not thread-safe; to be called only by worker. + /// Run network. Not thread-safe; to be called only by worker. virtual void doWork(); - /// Shutdown network. Called by Worker. Not thread-safe; to be called only by worker. + /// Shutdown network. Not thread-safe; to be called only by worker. virtual void doneWorking(); std::shared_ptr noteNode(NodeId _id, bi::tcp::endpoint _a, Origin _o, bool _ready, NodeId _oldId = NodeId());