diff --git a/libsolidity/AST.cpp b/libsolidity/AST.cpp index 2cb738d30..826673670 100644 --- a/libsolidity/AST.cpp +++ b/libsolidity/AST.cpp @@ -82,7 +82,7 @@ map, FunctionDefinition const*> ContractDefinition::getInterfaceFun FunctionDefinition const* ContractDefinition::getConstructor() const { for (ASTPointer const& f: m_definedFunctions) - if (f->getName() == getName()) + if (f->isConstructor()) return f.get(); return nullptr; } @@ -95,7 +95,7 @@ void ContractDefinition::checkIllegalOverrides() const for (ContractDefinition const* contract: getLinearizedBaseContracts()) for (ASTPointer const& function: contract->getDefinedFunctions()) { - if (function->getName() == contract->getName()) + if (function->isConstructor()) continue; // constructors can neither be overriden nor override anything FunctionDefinition const*& override = functions[function->getName()]; if (!override) @@ -115,8 +115,7 @@ vector, FunctionDefinition const*>> const& ContractDefinition: m_interfaceFunctionList.reset(new vector, FunctionDefinition const*>>()); for (ContractDefinition const* contract: getLinearizedBaseContracts()) for (ASTPointer const& f: contract->getDefinedFunctions()) - if (f->isPublic() && f->getName() != contract->getName() && - functionsSeen.count(f->getName()) == 0) + if (f->isPublic() && !f->isConstructor() && functionsSeen.count(f->getName()) == 0) { functionsSeen.insert(f->getName()); FixedHash<4> hash(dev::sha3(f->getCanonicalSignature())); diff --git a/libsolidity/AST.h b/libsolidity/AST.h index 8079348cf..754e9254c 100755 --- a/libsolidity/AST.h +++ b/libsolidity/AST.h @@ -281,12 +281,13 @@ class FunctionDefinition: public Declaration public: FunctionDefinition(Location const& _location, ASTPointer const& _name, bool _isPublic, + bool _isConstructor, ASTPointer const& _documentation, ASTPointer const& _parameters, bool _isDeclaredConst, ASTPointer const& _returnParameters, ASTPointer const& _body): - Declaration(_location, _name), m_isPublic(_isPublic), + Declaration(_location, _name), m_isPublic(_isPublic), m_isConstructor(_isConstructor), m_parameters(_parameters), m_isDeclaredConst(_isDeclaredConst), m_returnParameters(_returnParameters), @@ -298,6 +299,7 @@ public: virtual void accept(ASTConstVisitor& _visitor) const override; bool isPublic() const { return m_isPublic; } + bool isConstructor() const { return m_isConstructor; } bool isDeclaredConst() const { return m_isDeclaredConst; } std::vector> const& getParameters() const { return m_parameters->getParameters(); } ParameterList const& getParameterList() const { return *m_parameters; } @@ -321,6 +323,7 @@ public: private: bool m_isPublic; + bool m_isConstructor; ASTPointer m_parameters; bool m_isDeclaredConst; ASTPointer m_returnParameters; diff --git a/libsolidity/CallGraph.cpp b/libsolidity/CallGraph.cpp index 88d874f3b..a671796bd 100644 --- a/libsolidity/CallGraph.cpp +++ b/libsolidity/CallGraph.cpp @@ -38,6 +38,7 @@ void CallGraph::addNode(ASTNode const& _node) set const& CallGraph::getCalls() { + computeCallGraph(); return m_functionsSeen; } @@ -45,8 +46,7 @@ void CallGraph::computeCallGraph() { while (!m_workQueue.empty()) { - FunctionDefinition const* fun = m_workQueue.front(); - fun->accept(*this); + m_workQueue.front()->accept(*this); m_workQueue.pop(); } } @@ -55,7 +55,12 @@ bool CallGraph::visit(Identifier const& _identifier) { FunctionDefinition const* fun = dynamic_cast(_identifier.getReferencedDeclaration()); if (fun) + { + if (m_overrideResolver) + fun = (*m_overrideResolver)(fun->getName()); + solAssert(fun, "Error finding override for function " + fun->getName()); addFunction(*fun); + } return true; } diff --git a/libsolidity/CallGraph.h b/libsolidity/CallGraph.h index e3558fc25..90176e7e6 100644 --- a/libsolidity/CallGraph.h +++ b/libsolidity/CallGraph.h @@ -22,6 +22,7 @@ #include #include +#include #include #include @@ -38,8 +39,11 @@ namespace solidity class CallGraph: private ASTConstVisitor { public: + using OverrideResolver = std::function; + + CallGraph(OverrideResolver const& _overrideResolver): m_overrideResolver(&_overrideResolver) {} + void addNode(ASTNode const& _node); - void computeCallGraph(); std::set const& getCalls(); @@ -48,8 +52,10 @@ private: virtual bool visit(Identifier const& _identifier) override; virtual bool visit(MemberAccess const& _memberAccess) override; + void computeCallGraph(); void addFunction(FunctionDefinition const& _function); + OverrideResolver const* m_overrideResolver; std::set m_functionsSeen; std::queue m_workQueue; }; diff --git a/libsolidity/Compiler.cpp b/libsolidity/Compiler.cpp index 36316b9ae..5a434a71b 100644 --- a/libsolidity/Compiler.cpp +++ b/libsolidity/Compiler.cpp @@ -43,13 +43,13 @@ void Compiler::compileContract(ContractDefinition const& _contract, for (ContractDefinition const* contract: _contract.getLinearizedBaseContracts()) for (ASTPointer const& function: contract->getDefinedFunctions()) - if (function->getName() != contract->getName()) // don't add the constructor here + if (!function->isConstructor()) m_context.addFunction(*function); appendFunctionSelector(_contract); for (ContractDefinition const* contract: _contract.getLinearizedBaseContracts()) for (ASTPointer const& function: contract->getDefinedFunctions()) - if (function->getName() != contract->getName()) // don't add the constructor here + if (!function->isConstructor()) function->accept(*this); // Swap the runtime context with the creation-time context @@ -93,11 +93,30 @@ void Compiler::packIntoContractCreator(ContractDefinition const& _contract, Comp } } - //@TODO add virtual functions - neededFunctions = getFunctionsCalled(nodesUsedInConstructors); + auto overrideResolver = [&](string const& _name) -> FunctionDefinition const* + { + for (ContractDefinition const* contract: bases) + for (ASTPointer const& function: contract->getDefinedFunctions()) + if (!function->isConstructor() && function->getName() == _name) + return function.get(); + return nullptr; + }; + + neededFunctions = getFunctionsCalled(nodesUsedInConstructors, overrideResolver); + // First add all overrides (or the functions themselves if there is no override) + for (FunctionDefinition const* fun: neededFunctions) + { + FunctionDefinition const* override = nullptr; + if (!fun->isConstructor()) + override = overrideResolver(fun->getName()); + if (!!override && neededFunctions.count(override)) + m_context.addFunction(*override); + } + // now add the rest for (FunctionDefinition const* fun: neededFunctions) - m_context.addFunction(*fun); + if (fun->isConstructor() || overrideResolver(fun->getName()) != fun) + m_context.addFunction(*fun); // Call constructors in base-to-derived order. // The Constructor for the most derived contract is called later. @@ -159,10 +178,10 @@ void Compiler::appendConstructorCall(FunctionDefinition const& _constructor) m_context << returnTag; } -set Compiler::getFunctionsCalled(set const& _nodes) +set Compiler::getFunctionsCalled(set const& _nodes, + function const& _resolveOverrides) { - // TODO this does not add virtual functions - CallGraph callgraph; + CallGraph callgraph(_resolveOverrides); for (ASTNode const* node: _nodes) callgraph.addNode(*node); return callgraph.getCalls(); diff --git a/libsolidity/Compiler.h b/libsolidity/Compiler.h index ea05f38ee..2bae6b397 100644 --- a/libsolidity/Compiler.h +++ b/libsolidity/Compiler.h @@ -21,6 +21,7 @@ */ #include +#include #include #include @@ -49,7 +50,9 @@ private: std::vector> const& _arguments); void appendConstructorCall(FunctionDefinition const& _constructor); /// Recursively searches the call graph and returns all functions referenced inside _nodes. - std::set getFunctionsCalled(std::set const& _nodes); + /// _resolveOverride is called to resolve virtual function overrides. + std::set getFunctionsCalled(std::set const& _nodes, + std::function const& _resolveOverride); void appendFunctionSelector(ContractDefinition const& _contract); /// Creates code that unpacks the arguments for the given function, from memory if /// @a _fromMemory is true, otherwise from call data. @returns the size of the data in bytes. diff --git a/libsolidity/InterfaceHandler.cpp b/libsolidity/InterfaceHandler.cpp index 4c7ae5f45..1adce8cb8 100644 --- a/libsolidity/InterfaceHandler.cpp +++ b/libsolidity/InterfaceHandler.cpp @@ -351,9 +351,15 @@ void InterfaceHandler::parseDocString(std::string const& _string, CommentOwner _ currPos = appendDocTag(currPos, end, _owner); else if (currPos != end) { - if (nlPos == end) //end of text + // if it begins without a tag then consider it as @notice + if (currPos == _string.begin()) + { + currPos = parseDocTag(currPos, end, "notice", CommentOwner::FUNCTION); + continue; + } + else if (nlPos == end) //end of text return; - // else skip the line if a newline was found + // else skip the line if a newline was found and we get here currPos = nlPos + 1; } } diff --git a/libsolidity/Parser.cpp b/libsolidity/Parser.cpp index c0ca1abb2..fcabdb29b 100644 --- a/libsolidity/Parser.cpp +++ b/libsolidity/Parser.cpp @@ -112,9 +112,9 @@ ASTPointer Parser::parseImportDirective() ASTPointer Parser::parseContractDefinition() { ASTNodeFactory nodeFactory(*this); - ASTPointer docstring; + ASTPointer docString; if (m_scanner->getCurrentCommentLiteral() != "") - docstring = make_shared(m_scanner->getCurrentCommentLiteral()); + docString = make_shared(m_scanner->getCurrentCommentLiteral()); expectToken(Token::CONTRACT); ASTPointer name = expectIdentifierToken(); vector> baseContracts; @@ -142,7 +142,7 @@ ASTPointer Parser::parseContractDefinition() expectToken(Token::COLON); } else if (currentToken == Token::FUNCTION) - functions.push_back(parseFunctionDefinition(visibilityIsPublic)); + functions.push_back(parseFunctionDefinition(visibilityIsPublic, name.get())); else if (currentToken == Token::STRUCT) structs.push_back(parseStructDefinition()); else if (currentToken == Token::IDENTIFIER || currentToken == Token::MAPPING || @@ -157,7 +157,7 @@ ASTPointer Parser::parseContractDefinition() } nodeFactory.markEndPosition(); expectToken(Token::RBRACE); - return nodeFactory.createNode(name, docstring, baseContracts, structs, + return nodeFactory.createNode(name, docString, baseContracts, structs, stateVariables, functions); } @@ -178,7 +178,7 @@ ASTPointer Parser::parseInheritanceSpecifier() return nodeFactory.createNode(name, arguments); } -ASTPointer Parser::parseFunctionDefinition(bool _isPublic) +ASTPointer Parser::parseFunctionDefinition(bool _isPublic, ASTString const* _contractName) { ASTNodeFactory nodeFactory(*this); ASTPointer docstring; @@ -210,7 +210,8 @@ ASTPointer Parser::parseFunctionDefinition(bool _isPublic) } ASTPointer block = parseBlock(); nodeFactory.setEndPositionFromNode(block); - return nodeFactory.createNode(name, _isPublic, docstring, + bool const c_isConstructor = (_contractName && *name == *_contractName); + return nodeFactory.createNode(name, _isPublic, c_isConstructor, docstring, parameters, isDeclaredConst, returnParameters, block); } diff --git a/libsolidity/Parser.h b/libsolidity/Parser.h index 1b7a980ff..5905a0420 100644 --- a/libsolidity/Parser.h +++ b/libsolidity/Parser.h @@ -50,7 +50,7 @@ private: ASTPointer parseImportDirective(); ASTPointer parseContractDefinition(); ASTPointer parseInheritanceSpecifier(); - ASTPointer parseFunctionDefinition(bool _isPublic); + ASTPointer parseFunctionDefinition(bool _isPublic, ASTString const* _contractName); ASTPointer parseStructDefinition(); ASTPointer parseVariableDeclaration(bool _allowVar); ASTPointer parseTypeName(bool _allowVar); diff --git a/libsolidity/Types.cpp b/libsolidity/Types.cpp index c6d8b62fc..2446c513c 100644 --- a/libsolidity/Types.cpp +++ b/libsolidity/Types.cpp @@ -716,7 +716,7 @@ MemberList const& TypeType::getMembers() const // We are accessing the type of a base contract, so add all public and private // functions. Note that this does not add inherited functions on purpose. for (ASTPointer const& f: contract.getDefinedFunctions()) - if (f->getName() != contract.getName()) + if (!f->isConstructor()) members[f->getName()] = make_shared(*f); } m_members.reset(new MemberList(members)); diff --git a/neth/main.cpp b/neth/main.cpp index 638e9c624..2c9e7ec97 100644 --- a/neth/main.cpp +++ b/neth/main.cpp @@ -305,7 +305,6 @@ int main(int argc, char** argv) unsigned short remotePort = 30303; string dbPath; unsigned mining = ~(unsigned)0; - (void)mining; NodeMode mode = NodeMode::Full; unsigned peers = 5; #if ETH_JSONRPC @@ -441,6 +440,8 @@ int main(int argc, char** argv) web3.connect(Host::pocHost()); if (remoteHost.size()) web3.connect(remoteHost, remotePort); + if (mining) + c->startMining(); #if ETH_JSONRPC shared_ptr jsonrpcServer; diff --git a/test/SolidityEndToEndTest.cpp b/test/SolidityEndToEndTest.cpp index cba926d6d..cf04edaad 100644 --- a/test/SolidityEndToEndTest.cpp +++ b/test/SolidityEndToEndTest.cpp @@ -1610,6 +1610,28 @@ BOOST_AUTO_TEST_CASE(function_usage_in_constructor_arguments) BOOST_CHECK(callContractFunction("getA()") == encodeArgs(2)); } +BOOST_AUTO_TEST_CASE(virtual_function_usage_in_constructor_arguments) +{ + char const* sourceCode = R"( + contract BaseBase { + uint m_a; + function BaseBase(uint a) { + m_a = a; + } + function overridden() returns (uint r) { return 1; } + function g() returns (uint r) { return overridden(); } + } + contract Base is BaseBase(BaseBase.g()) { + } + contract Derived is Base() { + function getA() returns (uint r) { return m_a; } + function overridden() returns (uint r) { return 2; } + } + )"; + compileAndRun(sourceCode, 0, "Derived"); + BOOST_CHECK(callContractFunction("getA()") == encodeArgs(2)); +} + BOOST_AUTO_TEST_CASE(constructor_argument_overriding) { char const* sourceCode = R"( diff --git a/test/SolidityNatspecJSON.cpp b/test/SolidityNatspecJSON.cpp index 5cec0444c..743651d54 100644 --- a/test/SolidityNatspecJSON.cpp +++ b/test/SolidityNatspecJSON.cpp @@ -506,17 +506,35 @@ BOOST_AUTO_TEST_CASE(dev_title_at_function_error) BOOST_CHECK_THROW(checkNatspec(sourceCode, natspec, false), DocstringParsingError); } -// test for bug where having no tags in docstring would cause infinite loop -BOOST_AUTO_TEST_CASE(natspec_no_tags) +BOOST_AUTO_TEST_CASE(natspec_notice_without_tag) { char const* sourceCode = "contract test {\n" " /// I do something awesome\n" - " function mul(uint a, uint second) returns(uint d) { return a * 7 + second; }\n" + " function mul(uint a) returns(uint d) { return a * 7; }\n" "}\n"; - char const* natspec = "{\"methods\": {}}"; + char const* natspec = "{" + "\"methods\":{" + " \"mul(uint256)\":{ \"notice\": \"I do something awesome\"}" + "}}"; + + checkNatspec(sourceCode, natspec, true); +} - checkNatspec(sourceCode, natspec, false); +BOOST_AUTO_TEST_CASE(natspec_multiline_notice_without_tag) +{ + char const* sourceCode = "contract test {\n" + " /// I do something awesome\n" + " /// which requires two lines to explain\n" + " function mul(uint a) returns(uint d) { return a * 7; }\n" + "}\n"; + + char const* natspec = "{" + "\"methods\":{" + " \"mul(uint256)\":{ \"notice\": \"I do something awesome which requires two lines to explain\"}" + "}}"; + + checkNatspec(sourceCode, natspec, true); } BOOST_AUTO_TEST_SUITE_END()