From 5a6748e3df086acce91a895ceecdf45ed25d79f9 Mon Sep 17 00:00:00 2001 From: Lefteris Karapetsas Date: Wed, 21 Jan 2015 17:19:57 +0100 Subject: [PATCH 01/18] Parsing accessor functions for public contract state variables - During the contract parsing depending on whether or not a state variable is public an extra acessor FunctionDefinition is parsed for it --- libsolidity/Parser.cpp | 25 +++++++++++++++++++++++++ libsolidity/Parser.h | 4 ++++ 2 files changed, 29 insertions(+) diff --git a/libsolidity/Parser.cpp b/libsolidity/Parser.cpp index d99d33acc..cd395c77a 100644 --- a/libsolidity/Parser.cpp +++ b/libsolidity/Parser.cpp @@ -109,6 +109,29 @@ ASTPointer Parser::parseImportDirective() return nodeFactory.createNode(url); } +void Parser::addStateVariableAccessor(ASTPointer const& _varDecl, + vector> & _functions) +{ + ASTNodeFactory nodeFactory(*this); + ASTPointer emptyDoc; + ASTPointer emptyParamList; + nodeFactory.markEndPosition(); + auto expression = nodeFactory.createNode(make_shared(_varDecl->getName())); + vector> block_statements = {nodeFactory.createNode(expression)}; + + _functions.push_back(nodeFactory.createNode( + make_shared(_varDecl->getName()), + true, // isPublic + false, // not a Constructor + emptyDoc, // no documentation + emptyParamList, // no parameters (temporary, a mapping would need parameters for example) + true, // is constant + emptyParamList, // no return parameters + nodeFactory.createNode(block_statements) + ) + ); +} + ASTPointer Parser::parseContractDefinition() { ASTNodeFactory nodeFactory(*this); @@ -151,6 +174,8 @@ ASTPointer Parser::parseContractDefinition() { bool const allowVar = false; stateVariables.push_back(parseVariableDeclaration(allowVar)); + if (visibilityIsPublic) + addStateVariableAccessor(stateVariables.back(), functions); expectToken(Token::SEMICOLON); } else if (currentToken == Token::MODIFIER) diff --git a/libsolidity/Parser.h b/libsolidity/Parser.h index 211e952d3..d911598b6 100644 --- a/libsolidity/Parser.h +++ b/libsolidity/Parser.h @@ -78,6 +78,10 @@ private: ///@{ ///@name Helper functions + /// Depending on whether a state Variable is Public, appends an accessor to the contract's functions + void addStateVariableAccessor(ASTPointer const& _varDecl, + std::vector> & _functions); + /// Peeks ahead in the scanner to determine if a variable definition is going to follow bool peekVariableDefinition(); From cd677c09219316ba9d67fe38afafb35c7c0e0708 Mon Sep 17 00:00:00 2001 From: Lefteris Karapetsas Date: Wed, 21 Jan 2015 19:07:03 +0100 Subject: [PATCH 02/18] Fix in addStateVariableAccessor and adjustment of parser tests --- libsolidity/Parser.cpp | 9 +++++---- test/SolidityParser.cpp | 39 ++++++++++++++++++++++++--------------- 2 files changed, 29 insertions(+), 19 deletions(-) diff --git a/libsolidity/Parser.cpp b/libsolidity/Parser.cpp index cd395c77a..c9ab551e4 100644 --- a/libsolidity/Parser.cpp +++ b/libsolidity/Parser.cpp @@ -113,9 +113,10 @@ void Parser::addStateVariableAccessor(ASTPointer const& _va vector> & _functions) { ASTNodeFactory nodeFactory(*this); + nodeFactory.setLocationEmpty(); ASTPointer emptyDoc; - ASTPointer emptyParamList; - nodeFactory.markEndPosition(); + + vector> parameters; auto expression = nodeFactory.createNode(make_shared(_varDecl->getName())); vector> block_statements = {nodeFactory.createNode(expression)}; @@ -124,9 +125,9 @@ void Parser::addStateVariableAccessor(ASTPointer const& _va true, // isPublic false, // not a Constructor emptyDoc, // no documentation - emptyParamList, // no parameters (temporary, a mapping would need parameters for example) + nodeFactory.createNode(vector>()), true, // is constant - emptyParamList, // no return parameters + nodeFactory.createNode(vector>()), nodeFactory.createNode(block_statements) ) ); diff --git a/test/SolidityParser.cpp b/test/SolidityParser.cpp index e331b9c6d..db7806f4d 100644 --- a/test/SolidityParser.cpp +++ b/test/SolidityParser.cpp @@ -66,6 +66,14 @@ ASTPointer parseTextExplainError(std::string const& _source) } } +static void checkFunctionNatspec(ASTPointer _function, + std::string const& _expectedDoc) +{ + auto doc = _function->getDocumentation(); + BOOST_CHECK_MESSAGE(doc != nullptr, "Function does not have Natspec Doc as expected"); + BOOST_CHECK_EQUAL(*doc, _expectedDoc); +} + } @@ -121,14 +129,16 @@ BOOST_AUTO_TEST_CASE(function_natspec_documentation) ASTPointer contract; ASTPointer function; char const* text = "contract test {\n" - " uint256 stateVar;\n" + " private:\n" + " uint256 stateVar;\n" + " public:\n" " /// This is a test function\n" " function functionName(hash hashin) returns (hash hashout) {}\n" "}\n"; BOOST_REQUIRE_NO_THROW(contract = parseText(text)); auto functions = contract->getDefinedFunctions(); BOOST_REQUIRE_NO_THROW(function = functions.at(0)); - BOOST_CHECK_EQUAL(*function->getDocumentation(), "This is a test function"); + checkFunctionNatspec(function, "This is a test function"); } BOOST_AUTO_TEST_CASE(function_normal_comments) @@ -144,7 +154,7 @@ BOOST_AUTO_TEST_CASE(function_normal_comments) auto functions = contract->getDefinedFunctions(); BOOST_REQUIRE_NO_THROW(function = functions.at(0)); BOOST_CHECK_MESSAGE(function->getDocumentation() == nullptr, - "Should not have gotten a Natspect comment for this function"); + "Should not have gotten a Natspecc comment for this function"); } BOOST_AUTO_TEST_CASE(multiple_functions_natspec_documentation) @@ -152,7 +162,9 @@ BOOST_AUTO_TEST_CASE(multiple_functions_natspec_documentation) ASTPointer contract; ASTPointer function; char const* text = "contract test {\n" + " private:\n" " uint256 stateVar;\n" + " public:\n" " /// This is test function 1\n" " function functionName1(hash hashin) returns (hash hashout) {}\n" " /// This is test function 2\n" @@ -166,17 +178,17 @@ BOOST_AUTO_TEST_CASE(multiple_functions_natspec_documentation) auto functions = contract->getDefinedFunctions(); BOOST_REQUIRE_NO_THROW(function = functions.at(0)); - BOOST_CHECK_EQUAL(*function->getDocumentation(), "This is test function 1"); + checkFunctionNatspec(function, "This is test function 1"); BOOST_REQUIRE_NO_THROW(function = functions.at(1)); - BOOST_CHECK_EQUAL(*function->getDocumentation(), "This is test function 2"); + checkFunctionNatspec(function, "This is test function 2"); BOOST_REQUIRE_NO_THROW(function = functions.at(2)); BOOST_CHECK_MESSAGE(function->getDocumentation() == nullptr, "Should not have gotten natspec comment for functionName3()"); BOOST_REQUIRE_NO_THROW(function = functions.at(3)); - BOOST_CHECK_EQUAL(*function->getDocumentation(), "This is test function 4"); + checkFunctionNatspec(function, "This is test function 4"); } BOOST_AUTO_TEST_CASE(multiline_function_documentation) @@ -192,10 +204,9 @@ BOOST_AUTO_TEST_CASE(multiline_function_documentation) BOOST_REQUIRE_NO_THROW(contract = parseText(text)); auto functions = contract->getDefinedFunctions(); - BOOST_REQUIRE_NO_THROW(function = functions.at(0)); - BOOST_CHECK_EQUAL(*function->getDocumentation(), - "This is a test function\n" - " and it has 2 lines"); + BOOST_REQUIRE_NO_THROW(function = functions.at(1)); // 1 since, 0 is the index of stateVar accessor + checkFunctionNatspec(function, "This is a test function\n" + " and it has 2 lines"); } BOOST_AUTO_TEST_CASE(natspec_comment_in_function_body) @@ -211,7 +222,6 @@ BOOST_AUTO_TEST_CASE(natspec_comment_in_function_body) " mapping(address=>hash) d;\n" " string name = \"Solidity\";" " }\n" - " uint256 stateVar;\n" " /// This is a test function\n" " /// and it has 2 lines\n" " function fun(hash hashin) returns (hash hashout) {}\n" @@ -220,12 +230,11 @@ BOOST_AUTO_TEST_CASE(natspec_comment_in_function_body) auto functions = contract->getDefinedFunctions(); BOOST_REQUIRE_NO_THROW(function = functions.at(0)); - BOOST_CHECK_EQUAL(*function->getDocumentation(), "fun1 description"); + checkFunctionNatspec(function, "fun1 description"); BOOST_REQUIRE_NO_THROW(function = functions.at(1)); - BOOST_CHECK_EQUAL(*function->getDocumentation(), - "This is a test function\n" - " and it has 2 lines"); + checkFunctionNatspec(function, "This is a test function\n" + " and it has 2 lines"); } BOOST_AUTO_TEST_CASE(natspec_docstring_between_keyword_and_signature) From 818742dac91908c57332363a7e746fdaaf57e611 Mon Sep 17 00:00:00 2001 From: Lefteris Karapetsas Date: Thu, 22 Jan 2015 17:40:22 +0100 Subject: [PATCH 03/18] Work in progress for state variable accessors - Changed the code so that a generic declaration with the combination of a function type can be used wherer a function definition was used before - Since using an std::pair everywhere is really tiring with this commit I am in the process of abstracting it into a function --- libsolidity/AST.cpp | 48 +++++++++++++++++++++++++-------- libsolidity/AST.h | 35 +++++++++++++++++++----- libsolidity/Compiler.cpp | 46 +++++++++++++++---------------- libsolidity/Compiler.h | 8 +++--- libsolidity/CompilerContext.cpp | 4 +-- libsolidity/CompilerContext.h | 9 ++++--- libsolidity/Parser.cpp | 32 +++------------------- libsolidity/Parser.h | 6 +---- libsolidity/Types.cpp | 21 ++++++++++++--- libsolidity/Types.h | 3 ++- 10 files changed, 122 insertions(+), 90 deletions(-) diff --git a/libsolidity/AST.cpp b/libsolidity/AST.cpp index e5967caa5..7620eeece 100644 --- a/libsolidity/AST.cpp +++ b/libsolidity/AST.cpp @@ -68,19 +68,21 @@ void ContractDefinition::checkTypeRequirements() set> hashes; for (auto const& hashAndFunction: getInterfaceFunctionList()) { - FixedHash<4> const& hash = hashAndFunction.first; + FixedHash<4> const& hash = std::get<0>(hashAndFunction); if (hashes.count(hash)) - BOOST_THROW_EXCEPTION(createTypeError("Function signature hash collision for " + - hashAndFunction.second->getCanonicalSignature())); + BOOST_THROW_EXCEPTION(createTypeError( + "Function signature hash collision for " + + std::get<1>(hashAndFunction)>->getCanonicalSignature(std::get<2>(hashAndFunction)->getName()))); hashes.insert(hash); } } -map, FunctionDefinition const*> ContractDefinition::getInterfaceFunctions() const +map, pair> ContractDefinition::getInterfaceFunctions() const { - vector, FunctionDefinition const*>> exportedFunctionList = getInterfaceFunctionList(); - map, FunctionDefinition const*> exportedFunctions(exportedFunctionList.begin(), - exportedFunctionList.end()); + vector, FunctionType const*, Declaration const*>>> exportedFunctionList = getInterfaceFunctionList(); + map, pair> exportedFunctions(exportedFunctionList.begin(), + exportedFunctionList.end()); + solAssert(exportedFunctionList.size() == exportedFunctions.size(), "Hash collision at Function Definition Hash calculation"); @@ -134,20 +136,31 @@ void ContractDefinition::checkIllegalOverrides() const } } -vector, FunctionDefinition const*>> const& ContractDefinition::getInterfaceFunctionList() const +vector, FunctionType const*, Declaration const*>> const& ContractDefinition::getInterfaceFunctionList() const { if (!m_interfaceFunctionList) { set functionsSeen; - m_interfaceFunctionList.reset(new vector, FunctionDefinition const*>>()); + m_interfaceFunctionList.reset(new vector, FunctionType const*, Declaration const*>>()); for (ContractDefinition const* contract: getLinearizedBaseContracts()) + { for (ASTPointer const& f: contract->getDefinedFunctions()) if (f->isPublic() && !f->isConstructor() && functionsSeen.count(f->getName()) == 0) { functionsSeen.insert(f->getName()); FixedHash<4> hash(dev::sha3(f->getCanonicalSignature())); - m_interfaceFunctionList->push_back(make_pair(hash, f.get())); + m_interfaceFunctionList->push_back(make_tuple(hash, FunctionType(*f), f.get())); } + + for (ASTPointer const& v: contract->getStateVariables()) + if (v->isPublic()) + { + FunctionType ftype(*v); + functionsSeen.insert(v->getName()); + FixedHash<4> hash(dev::sha3(ftype.getCanonicalSignature(v->getName())); + m_interfaceFunctionList->push_back(make_tuple(hash, ftype, v.get())); + } + } } return *m_interfaceFunctionList; } @@ -219,7 +232,7 @@ void FunctionDefinition::checkTypeRequirements() string FunctionDefinition::getCanonicalSignature() const { - return getName() + FunctionType(*this).getCanonicalSignature(); + return FunctionType(*this).getCanonicalSignature(getName()); } Declaration::LValueType VariableDeclaration::getLValueType() const @@ -504,5 +517,18 @@ void Literal::checkTypeRequirements() BOOST_THROW_EXCEPTION(createTypeError("Invalid literal value.")); } + +ASTPointer FunctionDescription::getDocumentation() +{ + auto function = dynamic_cast(m_description.second); + if (function) + return function->getDocumentation(); +} + +string FunctionDescription::getSignature() +{ + return m_description.first->getCanonicalSignature(m_description.second->getName()); +} + } } diff --git a/libsolidity/AST.h b/libsolidity/AST.h index e047f9761..695e504ac 100755 --- a/libsolidity/AST.h +++ b/libsolidity/AST.h @@ -156,6 +156,23 @@ private: Declaration const* m_scope; }; + +/** + * Generic function description able to describe both normal functions and + * functions that should be made as accessors to state variables + */ +struct FunctionDescription +{ + FunctionDescription(FunctionType const *_type, Declaration const* _decl): + m_description(_type, _decl){} + + ASTPointer getDocumentation(); + std::string getSignature(); + + std::pair m_description; +}; + + /** * Definition of a contract. This is the only AST nodes where child nodes are not visited in * document order. It first visits all struct declarations, then all variable declarations and @@ -202,7 +219,7 @@ public: /// @returns a map of canonical function signatures to FunctionDefinitions /// as intended for use by the ABI. - std::map, FunctionDefinition const*> getInterfaceFunctions() const; + std::map, std::pair> getInterfaceFunctions() const; /// List of all (direct and indirect) base contracts in order from derived to base, including /// the contract itself. Available after name resolution @@ -215,7 +232,7 @@ public: private: void checkIllegalOverrides() const; - std::vector, FunctionDefinition const*>> const& getInterfaceFunctionList() const; + std::vector, FunctionType const*, Declaration const*>> const& getInterfaceFunctionList() const; std::vector> m_baseContracts; std::vector> m_definedStructs; @@ -225,7 +242,7 @@ private: ASTPointer m_documentation; std::vector m_linearizedBaseContracts; - mutable std::unique_ptr, FunctionDefinition const*>>> m_interfaceFunctionList; + mutable std::unique_ptr, FunctionType const*, Declaration const*>>> m_interfaceFunctionList; }; class InheritanceSpecifier: public ASTNode @@ -372,8 +389,8 @@ class VariableDeclaration: public Declaration { public: VariableDeclaration(Location const& _location, ASTPointer const& _type, - ASTPointer const& _name): - Declaration(_location, _name), m_typeName(_type) {} + ASTPointer const& _name, bool _isPublic): + Declaration(_location, _name), m_typeName(_type), m_isPublic(_isPublic) {} virtual void accept(ASTVisitor& _visitor) override; virtual void accept(ASTConstVisitor& _visitor) const override; @@ -385,10 +402,13 @@ public: void setType(std::shared_ptr const& _type) { m_type = _type; } virtual LValueType getLValueType() const override; + bool isLocalVariable() const { return !!dynamic_cast(getScope()); } + bool isPublic() const { return m_isPublic; } -private: - ASTPointer m_typeName; ///< can be empty ("var") +private: + ASTPointer m_typeName; ///< can be empty ("var") + bool m_isPublic; ///< Whether there is an accessor for it or not std::shared_ptr m_type; ///< derived type, initially empty }; @@ -1076,5 +1096,6 @@ private: /// @} + } } diff --git a/libsolidity/Compiler.cpp b/libsolidity/Compiler.cpp index 5190f93f3..13f8282e6 100644 --- a/libsolidity/Compiler.cpp +++ b/libsolidity/Compiler.cpp @@ -201,7 +201,7 @@ set Compiler::getFunctionsCalled(set void Compiler::appendFunctionSelector(ContractDefinition const& _contract) { - map, FunctionDefinition const*> interfaceFunctions = _contract.getInterfaceFunctions(); + map, FunctionType const*, Declaration const*> interfaceFunctions = _contract.getInterfaceFunctions(); map, const eth::AssemblyItem> callDataUnpackerEntryPoints; // retrieve the function signature hash from the calldata @@ -209,7 +209,6 @@ void Compiler::appendFunctionSelector(ContractDefinition const& _contract) CompilerUtils(m_context).loadFromMemory(0, 4, false, true); // stack now is: 1 0 - // for (auto it = interfaceFunctions.cbegin(); it != interfaceFunctions.cend(); ++it) for (auto const& it: interfaceFunctions) { callDataUnpackerEntryPoints.insert(std::make_pair(it.first, m_context.newTag())); @@ -220,29 +219,28 @@ void Compiler::appendFunctionSelector(ContractDefinition const& _contract) for (auto const& it: interfaceFunctions) { - FunctionDefinition const& function = *it.second; + FunctionType const* functionType = *it.second.first; m_context << callDataUnpackerEntryPoints.at(it.first); eth::AssemblyItem returnTag = m_context.pushNewTag(); - appendCalldataUnpacker(function); - m_context.appendJumpTo(m_context.getFunctionEntryLabel(function)); + appendCalldataUnpacker(functionType->getParameterTypes()); + m_context.appendJumpTo(m_context.getFunctionEntryLabel(it.second.second)); m_context << returnTag; - appendReturnValuePacker(function); + appendReturnValuePacker(functionType->getReturnParameterTypes()); } } -unsigned Compiler::appendCalldataUnpacker(FunctionDefinition const& _function, bool _fromMemory) +unsigned Compiler::appendCalldataUnpacker(TypePointers const& _typeParameters, bool _fromMemory) { // We do not check the calldata size, everything is zero-padded. unsigned dataOffset = CompilerUtils::dataStartOffset; // the 4 bytes of the function hash signature //@todo this can be done more efficiently, saving some CALLDATALOAD calls - for (ASTPointer const& var: _function.getParameters()) + for (TypePointer const& type: _typeParameters) { - unsigned const c_numBytes = var->getType()->getCalldataEncodedSize(); + unsigned const c_numBytes = type->getCalldataEncodedSize(); if (c_numBytes > 32) BOOST_THROW_EXCEPTION(CompilerError() - << errinfo_sourceLocation(var->getLocation()) - << errinfo_comment("Type " + var->getType()->toString() + " not yet supported.")); - bool const c_leftAligned = var->getType()->getCategory() == Type::Category::STRING; + << errinfo_comment("Type " + type->toString() + " not yet supported.")); + bool const c_leftAligned = type->getCategory() == Type::Category::STRING; bool const c_padToWords = true; dataOffset += CompilerUtils(m_context).loadFromMemory(dataOffset, c_numBytes, c_leftAligned, !_fromMemory, c_padToWords); @@ -250,26 +248,26 @@ unsigned Compiler::appendCalldataUnpacker(FunctionDefinition const& _function, b return dataOffset; } -void Compiler::appendReturnValuePacker(FunctionDefinition const& _function) +void Compiler::appendReturnValuePacker(TypePointers const& _typeParameters) { //@todo this can be also done more efficiently unsigned dataOffset = 0; - vector> const& parameters = _function.getReturnParameters(); - unsigned stackDepth = CompilerUtils(m_context).getSizeOnStack(parameters); - for (unsigned i = 0; i < parameters.size(); ++i) + unsigned stackDepth = 0; + for (TypePointer const& type: _typeParameters) + stackDepth += type->getSizeOnStack(); + + for (TypePointer const& type: _typeParameters) { - Type const& paramType = *parameters[i]->getType(); - unsigned numBytes = paramType.getCalldataEncodedSize(); + unsigned numBytes = type->getCalldataEncodedSize(); if (numBytes > 32) BOOST_THROW_EXCEPTION(CompilerError() - << errinfo_sourceLocation(parameters[i]->getLocation()) - << errinfo_comment("Type " + paramType.toString() + " not yet supported.")); - CompilerUtils(m_context).copyToStackTop(stackDepth, paramType); - ExpressionCompiler::appendTypeConversion(m_context, paramType, paramType, true); - bool const c_leftAligned = paramType.getCategory() == Type::Category::STRING; + << errinfo_comment("Type " + type->toString() + " not yet supported.")); + CompilerUtils(m_context).copyToStackTop(stackDepth, *type); + ExpressionCompiler::appendTypeConversion(m_context, *type, *type, true); + bool const c_leftAligned = type->getCategory() == Type::Category::STRING; bool const c_padToWords = true; dataOffset += CompilerUtils(m_context).storeInMemory(dataOffset, numBytes, c_leftAligned, c_padToWords); - stackDepth -= paramType.getSizeOnStack(); + stackDepth -= type->getSizeOnStack(); } // note that the stack is not cleaned up here m_context << u256(dataOffset) << u256(0) << eth::Instruction::RETURN; diff --git a/libsolidity/Compiler.h b/libsolidity/Compiler.h index b65476460..f40339b03 100644 --- a/libsolidity/Compiler.h +++ b/libsolidity/Compiler.h @@ -56,10 +56,10 @@ private: std::function const& _resolveFunctionOverride, std::function const& _resolveModifierOverride); 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. - unsigned appendCalldataUnpacker(FunctionDefinition const& _function, bool _fromMemory = false); - void appendReturnValuePacker(FunctionDefinition const& _function); + /// Creates code that unpacks the arguments for the given function represented by a vector of TypePointers. + /// From memory if @a _fromMemory is true, otherwise from call data. @returns the size of the data in bytes. + unsigned appendCalldataUnpacker(TypePointers const& _typeParameters, bool _fromMemory = false); + void appendReturnValuePacker(TypePointers const& _typeParameters); void registerStateVariables(ContractDefinition const& _contract); diff --git a/libsolidity/CompilerContext.cpp b/libsolidity/CompilerContext.cpp index ad1877ba6..4edced940 100644 --- a/libsolidity/CompilerContext.cpp +++ b/libsolidity/CompilerContext.cpp @@ -83,9 +83,9 @@ bool CompilerContext::isLocalVariable(Declaration const* _declaration) const return m_localVariables.count(_declaration); } -eth::AssemblyItem CompilerContext::getFunctionEntryLabel(FunctionDefinition const& _function) const +eth::AssemblyItem CompilerContext::getFunctionEntryLabel(Declaration const& _declaration) const { - auto res = m_functionEntryLabels.find(&_function); + auto res = m_functionEntryLabels.find(&_declaration); solAssert(res != m_functionEntryLabels.end(), "Function entry label not found."); return res->second.tag(); } diff --git a/libsolidity/CompilerContext.h b/libsolidity/CompilerContext.h index d82dfe51e..aa438cf02 100644 --- a/libsolidity/CompilerContext.h +++ b/libsolidity/CompilerContext.h @@ -58,7 +58,7 @@ public: bool isLocalVariable(Declaration const* _declaration) const; bool isStateVariable(Declaration const* _declaration) const { return m_stateVariables.count(_declaration) != 0; } - eth::AssemblyItem getFunctionEntryLabel(FunctionDefinition const& _function) const; + eth::AssemblyItem getFunctionEntryLabel(Declaration const& _declaration) const; /// @returns the entry label of the given function and takes overrides into account. eth::AssemblyItem getVirtualFunctionEntryLabel(FunctionDefinition const& _function) const; ModifierDefinition const& getFunctionModifier(std::string const& _name) const; @@ -115,9 +115,12 @@ private: u256 m_stateVariablesSize = 0; /// Storage offsets of state variables std::map m_stateVariables; - /// Positions of local variables on the stack. + /// Offsets of local variables on the stack (relative to stack base). std::map m_localVariables; - /// Labels pointing to the entry points of funcitons. + /// Sum of stack sizes of local variables + unsigned m_localVariablesSize; + /// Labels pointing to the entry points of functions. + std::map m_functionEntryLabels; /// Labels pointing to the entry points of function overrides. std::map m_virtualFunctionEntryLabels; diff --git a/libsolidity/Parser.cpp b/libsolidity/Parser.cpp index c9ab551e4..1c61aab12 100644 --- a/libsolidity/Parser.cpp +++ b/libsolidity/Parser.cpp @@ -109,30 +109,6 @@ ASTPointer Parser::parseImportDirective() return nodeFactory.createNode(url); } -void Parser::addStateVariableAccessor(ASTPointer const& _varDecl, - vector> & _functions) -{ - ASTNodeFactory nodeFactory(*this); - nodeFactory.setLocationEmpty(); - ASTPointer emptyDoc; - - vector> parameters; - auto expression = nodeFactory.createNode(make_shared(_varDecl->getName())); - vector> block_statements = {nodeFactory.createNode(expression)}; - - _functions.push_back(nodeFactory.createNode( - make_shared(_varDecl->getName()), - true, // isPublic - false, // not a Constructor - emptyDoc, // no documentation - nodeFactory.createNode(vector>()), - true, // is constant - nodeFactory.createNode(vector>()), - nodeFactory.createNode(block_statements) - ) - ); -} - ASTPointer Parser::parseContractDefinition() { ASTNodeFactory nodeFactory(*this); @@ -174,9 +150,7 @@ ASTPointer Parser::parseContractDefinition() Token::isElementaryTypeName(currentToken)) { bool const allowVar = false; - stateVariables.push_back(parseVariableDeclaration(allowVar)); - if (visibilityIsPublic) - addStateVariableAccessor(stateVariables.back(), functions); + stateVariables.push_back(parseVariableDeclaration(allowVar, visibilityIsPublic)); expectToken(Token::SEMICOLON); } else if (currentToken == Token::MODIFIER) @@ -271,12 +245,12 @@ ASTPointer Parser::parseStructDefinition() return nodeFactory.createNode(name, members); } -ASTPointer Parser::parseVariableDeclaration(bool _allowVar) +ASTPointer Parser::parseVariableDeclaration(bool _allowVar, bool _isPublic) { ASTNodeFactory nodeFactory(*this); ASTPointer type = parseTypeName(_allowVar); nodeFactory.markEndPosition(); - return nodeFactory.createNode(type, expectIdentifierToken()); + return nodeFactory.createNode(type, expectIdentifierToken(), _isPublic); } ASTPointer Parser::parseModifierDefinition() diff --git a/libsolidity/Parser.h b/libsolidity/Parser.h index d911598b6..a8f97c6e3 100644 --- a/libsolidity/Parser.h +++ b/libsolidity/Parser.h @@ -52,7 +52,7 @@ private: ASTPointer parseInheritanceSpecifier(); ASTPointer parseFunctionDefinition(bool _isPublic, ASTString const* _contractName); ASTPointer parseStructDefinition(); - ASTPointer parseVariableDeclaration(bool _allowVar); + ASTPointer parseVariableDeclaration(bool _allowVar, bool _isPublic = false); ASTPointer parseModifierDefinition(); ASTPointer parseModifierInvocation(); ASTPointer parseIdentifier(); @@ -78,10 +78,6 @@ private: ///@{ ///@name Helper functions - /// Depending on whether a state Variable is Public, appends an accessor to the contract's functions - void addStateVariableAccessor(ASTPointer const& _varDecl, - std::vector> & _functions); - /// Peeks ahead in the scanner to determine if a variable definition is going to follow bool peekVariableDefinition(); diff --git a/libsolidity/Types.cpp b/libsolidity/Types.cpp index dad3a1455..d5b001636 100644 --- a/libsolidity/Types.cpp +++ b/libsolidity/Types.cpp @@ -489,7 +489,7 @@ MemberList const& ContractType::getMembers() const map> members(IntegerType::AddressMemberList.begin(), IntegerType::AddressMemberList.end()); for (auto const& it: m_contract.getInterfaceFunctions()) - members[it.second->getName()] = make_shared(*it.second, false); + members[it.second.second->getName()] = make_shared(*it.second.second, false); m_members.reset(new MemberList(members)); } return *m_members; @@ -512,7 +512,7 @@ u256 ContractType::getFunctionIdentifier(string const& _functionName) const { auto interfaceFunctions = m_contract.getInterfaceFunctions(); for (auto it = interfaceFunctions.cbegin(); it != interfaceFunctions.cend(); ++it) - if (it->second->getName() == _functionName) + if (it->second.second->getName() == _functionName) return FixedHash<4>::Arith(it->first); return Invalid256; @@ -593,6 +593,19 @@ FunctionType::FunctionType(FunctionDefinition const& _function, bool _isInternal swap(retParams, m_returnParameterTypes); } +FunctionType::FunctionType(VariableDeclaration const& _varDecl): + m_location(Location::INTERNAL) +{ + TypePointers params; + TypePointers retParams; + // for now, no input parameters LTODO: change for some things like mapping + params.reserve(0); + retParams.reserve(1); + retParams.push_back(_varDecl.getType()); + swap(params, m_parameterTypes); + swap(retParams, m_returnParameterTypes); +} + bool FunctionType::operator==(Type const& _other) const { if (_other.getCategory() != getCategory()) @@ -672,9 +685,9 @@ MemberList const& FunctionType::getMembers() const } } -string FunctionType::getCanonicalSignature() const +string FunctionType::getCanonicalSignature(std::string const& _name) const { - string ret = "("; + string ret = _name + "("; for (auto it = m_parameterTypes.cbegin(); it != m_parameterTypes.cend(); ++it) ret += (*it)->toString() + (it + 1 == m_parameterTypes.cend() ? "" : ","); diff --git a/libsolidity/Types.h b/libsolidity/Types.h index b7f87d760..b3a92021b 100644 --- a/libsolidity/Types.h +++ b/libsolidity/Types.h @@ -353,6 +353,7 @@ public: virtual Category getCategory() const override { return Category::FUNCTION; } explicit FunctionType(FunctionDefinition const& _function, bool _isInternal = true); + explicit FunctionType(VariableDeclaration const& _varDecl); FunctionType(strings const& _parameterTypes, strings const& _returnParameterTypes, Location _location = Location::INTERNAL): FunctionType(parseElementaryTypeVector(_parameterTypes), parseElementaryTypeVector(_returnParameterTypes), @@ -375,7 +376,7 @@ public: virtual MemberList const& getMembers() const override; Location const& getLocation() const { return m_location; } - std::string getCanonicalSignature() const; + std::string getCanonicalSignature(std::string const &_name) const; bool gasSet() const { return m_gasSet; } bool valueSet() const { return m_valueSet; } From 2874d4323840ec7eb2f97f1fc839472c4a9af37a Mon Sep 17 00:00:00 2001 From: Lefteris Karapetsas Date: Thu, 22 Jan 2015 17:43:16 +0100 Subject: [PATCH 04/18] Tests for variable state accessors are in progress --- test/SolidityNameAndTypeResolution.cpp | 62 +++++++++++++++++++++++++- 1 file changed, 61 insertions(+), 1 deletion(-) diff --git a/test/SolidityNameAndTypeResolution.cpp b/test/SolidityNameAndTypeResolution.cpp index 3720b3cde..7728414de 100644 --- a/test/SolidityNameAndTypeResolution.cpp +++ b/test/SolidityNameAndTypeResolution.cpp @@ -23,6 +23,7 @@ #include #include +#include #include #include #include @@ -53,6 +54,48 @@ ASTPointer parseTextAndResolveNames(std::string const& _source) return sourceUnit; } + +ASTPointer parseTextAndResolveNamesWithChecks(std::string const& _source) +{ + Parser parser; + ASTPointer sourceUnit; + try + { + sourceUnit = parser.parse(std::make_shared(CharStream(_source))); + NameAndTypeResolver resolver({}); + resolver.registerDeclarations(*sourceUnit); + for (ASTPointer const& node: sourceUnit->getNodes()) + if (ContractDefinition* contract = dynamic_cast(node.get())) + resolver.resolveNamesAndTypes(*contract); + for (ASTPointer const& node: sourceUnit->getNodes()) + if (ContractDefinition* contract = dynamic_cast(node.get())) + resolver.checkTypeRequirements(*contract); + } + catch(boost::exception const& _e) + { + auto msg = std::string("Parsing text and resolving nanes failed with: \n") + boost::diagnostic_information(_e); + BOOST_FAIL(msg); + } + return sourceUnit; +} + +static ContractDefinition const* retrieveContract(ASTPointer _source, unsigned index) +{ + ContractDefinition* contract; + unsigned counter = 0; + for (ASTPointer const& node: _source->getNodes()) + if ((contract = dynamic_cast(node.get())) && counter == index) + return contract; + + return NULL; +} + +static FunctionDefinition const* retrieveFunctionBySignature(ContractDefinition const* _contract, + std::string const& _signature) +{ + FixedHash<4> hash(dev::sha3(_signature)); + return _contract->getInterfaceFunctions()[hash]; +} } BOOST_AUTO_TEST_SUITE(SolidityNameAndTypeResolution) @@ -63,7 +106,7 @@ BOOST_AUTO_TEST_CASE(smoke_test) " uint256 stateVariable1;\n" " function fun(uint256 arg1) { var x; uint256 y; }" "}\n"; - BOOST_CHECK_NO_THROW(parseTextAndResolveNames(text)); + BOOST_CHECK_NO_THROW(parseTextAndResolveNamesWithChecks(text)); } BOOST_AUTO_TEST_CASE(double_stateVariable_declaration) @@ -584,6 +627,23 @@ BOOST_AUTO_TEST_CASE(modifier_returns_value) BOOST_CHECK_THROW(parseTextAndResolveNames(text), TypeError); } +BOOST_AUTO_TEST_CASE(state_variable_accessors) +{ + char const* text = "contract base {\n" + " function fun() {\n" + " uint64(2);\n" + " }\n" + "uint256 foo;\n" + "}\n"; + + ASTPointer source; + ContractDefinition const* contract; + FunctionDefinition const* function; + BOOST_CHECK_NO_THROW(source = parseTextAndResolveNamesWithChecks(text)); + BOOST_CHECK((contract = retrieveContract(source, 0)) != nullptr); + BOOST_CHECK((function = retrieveFunctionBySignature(contract, "foo()")) != nullptr); +} + BOOST_AUTO_TEST_SUITE_END() } From 06764f026efc47c6d28aa42627b09a1a1515ac7e Mon Sep 17 00:00:00 2001 From: Lefteris Karapetsas Date: Fri, 23 Jan 2015 16:37:06 +0100 Subject: [PATCH 05/18] State variable accessors code is now more organized - FunctionDescription is the abstraction of what should describe a function. It can either be a VariableDeclaration of a FunctionDefinition. - ParamDescription is what FunctionDescription uses to describe its parameters for outside use purposes with a pair of (name, type) strings - Modified code around Solidity and especially interface handler to adapt to this change --- alethzero/MainWin.cpp | 2 +- libsolidity/AST.cpp | 115 ++++++++++++++++++++++--- libsolidity/AST.h | 54 ++++++++++-- libsolidity/Compiler.cpp | 9 +- libsolidity/InterfaceHandler.cpp | 37 ++++---- libsolidity/Types.cpp | 27 +++++- libsolidity/Types.h | 4 + test/SolidityNameAndTypeResolution.cpp | 34 ++++++-- 8 files changed, 229 insertions(+), 53 deletions(-) diff --git a/alethzero/MainWin.cpp b/alethzero/MainWin.cpp index 2d7146426..103dfbb33 100644 --- a/alethzero/MainWin.cpp +++ b/alethzero/MainWin.cpp @@ -1674,7 +1674,7 @@ string const Main::getFunctionHashes(dev::solidity::CompilerStack const &_compil { ret += it.first.abridged(); ret += " :"; - ret += it.second->getName() + "\n"; + ret += it.second.getName() + "\n"; } return ret; } diff --git a/libsolidity/AST.cpp b/libsolidity/AST.cpp index 7620eeece..e4b5ed7d3 100644 --- a/libsolidity/AST.cpp +++ b/libsolidity/AST.cpp @@ -71,17 +71,19 @@ void ContractDefinition::checkTypeRequirements() FixedHash<4> const& hash = std::get<0>(hashAndFunction); if (hashes.count(hash)) BOOST_THROW_EXCEPTION(createTypeError( - "Function signature hash collision for " + - std::get<1>(hashAndFunction)>->getCanonicalSignature(std::get<2>(hashAndFunction)->getName()))); + std::string("Function signature hash collision for ") + + std::get<1>(hashAndFunction)->getCanonicalSignature(std::get<2>(hashAndFunction)->getName()))); hashes.insert(hash); } } -map, pair> ContractDefinition::getInterfaceFunctions() const +map, FunctionDescription> ContractDefinition::getInterfaceFunctions() const { - vector, FunctionType const*, Declaration const*>>> exportedFunctionList = getInterfaceFunctionList(); - map, pair> exportedFunctions(exportedFunctionList.begin(), - exportedFunctionList.end()); + auto exportedFunctionList = getInterfaceFunctionList(); + + map, FunctionDescription> exportedFunctions; + for (auto const& it: exportedFunctionList) + exportedFunctions[std::get<0>(it)] = std::move(FunctionDescription(std::get<1>(it), std::get<2>(it))); solAssert(exportedFunctionList.size() == exportedFunctions.size(), "Hash collision at Function Definition Hash calculation"); @@ -136,12 +138,12 @@ void ContractDefinition::checkIllegalOverrides() const } } -vector, FunctionType const*, Declaration const*>> const& ContractDefinition::getInterfaceFunctionList() const +vector, std::shared_ptr, Declaration const*>> const& ContractDefinition::getInterfaceFunctionList() const { if (!m_interfaceFunctionList) { set functionsSeen; - m_interfaceFunctionList.reset(new vector, FunctionType const*, Declaration const*>>()); + m_interfaceFunctionList.reset(new vector, std::shared_ptr, Declaration const*>>()); for (ContractDefinition const* contract: getLinearizedBaseContracts()) { for (ASTPointer const& f: contract->getDefinedFunctions()) @@ -149,7 +151,7 @@ vector, FunctionType const*, Declaration const*>> const& Cont { functionsSeen.insert(f->getName()); FixedHash<4> hash(dev::sha3(f->getCanonicalSignature())); - m_interfaceFunctionList->push_back(make_tuple(hash, FunctionType(*f), f.get())); + m_interfaceFunctionList->push_back(make_tuple(hash, make_shared(*f), f.get())); } for (ASTPointer const& v: contract->getStateVariables()) @@ -157,8 +159,8 @@ vector, FunctionType const*, Declaration const*>> const& Cont { FunctionType ftype(*v); functionsSeen.insert(v->getName()); - FixedHash<4> hash(dev::sha3(ftype.getCanonicalSignature(v->getName())); - m_interfaceFunctionList->push_back(make_tuple(hash, ftype, v.get())); + FixedHash<4> hash(dev::sha3(ftype.getCanonicalSignature(v->getName()))); + m_interfaceFunctionList->push_back(make_tuple(hash, make_shared(*v), v.get())); } } } @@ -518,17 +520,104 @@ void Literal::checkTypeRequirements() } -ASTPointer FunctionDescription::getDocumentation() +bool ParamDescription::operator!=(ParamDescription const& _other) const +{ + return m_description.first == _other.getName() && m_description.second == _other.getType(); +} + +std::ostream& ParamDescription::operator<<(std::ostream& os) const +{ + return os << m_description.first << ":" << m_description.second; +} + +std::string ParamDescription::getName() const +{ + return m_description.first; +} + +std::string ParamDescription::getType() const +{ + return m_description.second; +} + + +ASTPointer FunctionDescription::getDocumentation() const { auto function = dynamic_cast(m_description.second); if (function) return function->getDocumentation(); + + return ASTPointer(); } -string FunctionDescription::getSignature() +string FunctionDescription::getSignature() const { return m_description.first->getCanonicalSignature(m_description.second->getName()); } +string FunctionDescription::getName() const +{ + return m_description.second->getName(); +} + +bool FunctionDescription::isConstant() const +{ + auto function = dynamic_cast(m_description.second); + if (function) + return function->isDeclaredConst(); + + return true; +} + +vector const FunctionDescription::getParameters() const +{ + auto function = dynamic_cast(m_description.second); + if (function) + { + vector paramsDescription; + for (auto const& param: function->getParameters()) + paramsDescription.push_back(ParamDescription(param->getName(), param->getType()->toString())); + + return paramsDescription; + } + + // else for now let's assume no parameters to accessors + // LTODO: fix this for mapping types + return {}; +} + +vector const FunctionDescription::getReturnParameters() const +{ + auto function = dynamic_cast(m_description.second); + if (function) + { + vector paramsDescription; + for (auto const& param: function->getParameters()) + paramsDescription.push_back(ParamDescription(param->getName(), param->getType()->toString())); + + return paramsDescription; + } + + auto vardecl = dynamic_cast(m_description.second); + return {ParamDescription(vardecl->getName(), vardecl->getType()->toString())}; +} + +Declaration const* FunctionDescription::getDeclaration() const +{ + return m_description.second; +} + +shared_ptr FunctionDescription::getFunctionTypeShared() const +{ + return m_description.first; +} + + +FunctionType const* FunctionDescription::getFunctionType() const +{ + return m_description.first.get(); +} + + } } diff --git a/libsolidity/AST.h b/libsolidity/AST.h index 695e504ac..89daf80fb 100755 --- a/libsolidity/AST.h +++ b/libsolidity/AST.h @@ -157,19 +157,59 @@ private: }; +/** +* Generic Parameter description used by @see FunctionDescription to return +* a descripton of its parameters. +*/ +struct ParamDescription +{ + ParamDescription(std::string const& _name, std::string const& _type): + m_description(_name, _type){} + + bool operator!=(ParamDescription const& _other) const; + std::ostream& operator<<(std::ostream& os) const; + + std::string getName() const; + std::string getType() const; + + std::pair m_description; +}; + + /** * Generic function description able to describe both normal functions and * functions that should be made as accessors to state variables */ struct FunctionDescription { - FunctionDescription(FunctionType const *_type, Declaration const* _decl): + FunctionDescription(std::shared_ptr _type, Declaration const* _decl): m_description(_type, _decl){} - ASTPointer getDocumentation(); - std::string getSignature(); + FunctionDescription(): + m_description(nullptr, nullptr){} - std::pair m_description; + /// @returns the natspec documentation of the function if existing. Accessor (for now) don't have natspec doc + ASTPointer getDocumentation() const; + /// @returns the canonical signature of the function + std::string getSignature() const; + /// @returns the name of the function, basically that of the declaration + std::string getName() const; + /// @returns whether the function is constant. IF it's an accessor this is always true + bool isConstant() const; + /// @returns the argument parameters of the function + std::vector const getParameters() const; + /// @returns the return parameters of the function + std::vector const getReturnParameters() const; + /// @returns the Declaration AST Node pointer + Declaration const* getDeclaration() const; + /// @returns a created shared pointer with the type of the function + std::shared_ptr makeFunctionType() const; + /// @returns a pointer to the function type + FunctionType const* getFunctionType() const; + /// @returns a shared pointer to the function type + std::shared_ptr getFunctionTypeShared() const; + + std::pair, Declaration const*> m_description; }; @@ -219,7 +259,7 @@ public: /// @returns a map of canonical function signatures to FunctionDefinitions /// as intended for use by the ABI. - std::map, std::pair> getInterfaceFunctions() const; + std::map, FunctionDescription> getInterfaceFunctions() const; /// List of all (direct and indirect) base contracts in order from derived to base, including /// the contract itself. Available after name resolution @@ -232,7 +272,7 @@ public: private: void checkIllegalOverrides() const; - std::vector, FunctionType const*, Declaration const*>> const& getInterfaceFunctionList() const; + std::vector, std::shared_ptr, Declaration const*>> const& getInterfaceFunctionList() const; std::vector> m_baseContracts; std::vector> m_definedStructs; @@ -242,7 +282,7 @@ private: ASTPointer m_documentation; std::vector m_linearizedBaseContracts; - mutable std::unique_ptr, FunctionType const*, Declaration const*>>> m_interfaceFunctionList; + mutable std::unique_ptr, std::shared_ptr, Declaration const*>>> m_interfaceFunctionList; }; class InheritanceSpecifier: public ASTNode diff --git a/libsolidity/Compiler.cpp b/libsolidity/Compiler.cpp index 13f8282e6..f6f48a8c2 100644 --- a/libsolidity/Compiler.cpp +++ b/libsolidity/Compiler.cpp @@ -177,13 +177,14 @@ void Compiler::appendConstructorCall(FunctionDefinition const& _constructor) unsigned argumentSize = 0; for (ASTPointer const& var: _constructor.getParameters()) argumentSize += CompilerUtils::getPaddedSize(var->getType()->getCalldataEncodedSize()); + if (argumentSize > 0) { m_context << u256(argumentSize); m_context.appendProgramSize(); m_context << u256(CompilerUtils::dataStartOffset); // copy it to byte four as expected for ABI calls m_context << eth::Instruction::CODECOPY; - appendCalldataUnpacker(_constructor, true); + appendCalldataUnpacker(FunctionType(_constructor).getParameterTypes(), true); } m_context.appendJumpTo(m_context.getFunctionEntryLabel(_constructor)); m_context << returnTag; @@ -201,7 +202,7 @@ set Compiler::getFunctionsCalled(set void Compiler::appendFunctionSelector(ContractDefinition const& _contract) { - map, FunctionType const*, Declaration const*> interfaceFunctions = _contract.getInterfaceFunctions(); + map, FunctionDescription> interfaceFunctions = _contract.getInterfaceFunctions(); map, const eth::AssemblyItem> callDataUnpackerEntryPoints; // retrieve the function signature hash from the calldata @@ -219,11 +220,11 @@ void Compiler::appendFunctionSelector(ContractDefinition const& _contract) for (auto const& it: interfaceFunctions) { - FunctionType const* functionType = *it.second.first; + FunctionType const* functionType = it.second.getFunctionType(); m_context << callDataUnpackerEntryPoints.at(it.first); eth::AssemblyItem returnTag = m_context.pushNewTag(); appendCalldataUnpacker(functionType->getParameterTypes()); - m_context.appendJumpTo(m_context.getFunctionEntryLabel(it.second.second)); + m_context.appendJumpTo(m_context.getFunctionEntryLabel(*it.second.getDeclaration())); m_context << returnTag; appendReturnValuePacker(functionType->getReturnParameterTypes()); } diff --git a/libsolidity/InterfaceHandler.cpp b/libsolidity/InterfaceHandler.cpp index 1adce8cb8..9b6327782 100644 --- a/libsolidity/InterfaceHandler.cpp +++ b/libsolidity/InterfaceHandler.cpp @@ -45,23 +45,23 @@ std::unique_ptr InterfaceHandler::getABIInterface(ContractDefinitio Json::Value inputs(Json::arrayValue); Json::Value outputs(Json::arrayValue); - auto populateParameters = [](std::vector> const& _vars) + auto populateParameters = [](vector const& _params) { Json::Value params(Json::arrayValue); - for (ASTPointer const& var: _vars) + for (auto const& param: _params) { Json::Value input; - input["name"] = var->getName(); - input["type"] = var->getType()->toString(); + input["name"] = param.getName(); + input["type"] = param.getType(); params.append(input); } return params; }; - method["name"] = it.second->getName(); - method["constant"] = it.second->isDeclaredConst(); - method["inputs"] = populateParameters(it.second->getParameters()); - method["outputs"] = populateParameters(it.second->getReturnParameters()); + method["name"] = it.second.getName(); + method["constant"] = it.second.isConstant(); + method["inputs"] = populateParameters(it.second.getParameters()); + method["outputs"] = populateParameters(it.second.getReturnParameters()); methods.append(method); } return std::unique_ptr(new std::string(m_writer.write(methods))); @@ -72,17 +72,16 @@ unique_ptr InterfaceHandler::getABISolidityInterface(ContractDefinition string ret = "contract " + _contractDef.getName() + "{"; for (auto const& it: _contractDef.getInterfaceFunctions()) { - FunctionDefinition const* f = it.second; - auto populateParameters = [](vector> const& _vars) + auto populateParameters = [](vector const& _params) { string r = ""; - for (ASTPointer const& var: _vars) - r += (r.size() ? "," : "(") + var->getType()->toString() + " " + var->getName(); + for (auto const& param: _params) + r += (r.size() ? "," : "(") + param.getType() + " " + param.getName(); return r.size() ? r + ")" : "()"; }; - ret += "function " + f->getName() + populateParameters(f->getParameters()) + (f->isDeclaredConst() ? "constant " : ""); - if (f->getReturnParameters().size()) - ret += "returns" + populateParameters(f->getReturnParameters()); + ret += "function " + it.second.getName() + populateParameters(it.second.getParameters()) + (it.second.isConstant() ? "constant " : ""); + if (it.second.getReturnParameters().size()) + ret += "returns" + populateParameters(it.second.getReturnParameters()); else if (ret.back() == ' ') ret.pop_back(); ret += "{}"; @@ -98,7 +97,7 @@ std::unique_ptr InterfaceHandler::getUserDocumentation(ContractDefi for (auto const& it: _contractDef.getInterfaceFunctions()) { Json::Value user; - auto strPtr = it.second->getDocumentation(); + auto strPtr = it.second.getDocumentation(); if (strPtr) { resetUser(); @@ -106,7 +105,7 @@ std::unique_ptr InterfaceHandler::getUserDocumentation(ContractDefi if (!m_notice.empty()) {// since @notice is the only user tag if missing function should not appear user["notice"] = Json::Value(m_notice); - methods[it.second->getCanonicalSignature()] = user; + methods[it.second.getSignature()] = user; } } } @@ -139,7 +138,7 @@ std::unique_ptr InterfaceHandler::getDevDocumentation(ContractDefin for (auto const& it: _contractDef.getInterfaceFunctions()) { Json::Value method; - auto strPtr = it.second->getDocumentation(); + auto strPtr = it.second.getDocumentation(); if (strPtr) { resetDev(); @@ -162,7 +161,7 @@ std::unique_ptr InterfaceHandler::getDevDocumentation(ContractDefin method["return"] = m_return; if (!method.empty()) // add the function, only if we have any documentation to add - methods[it.second->getCanonicalSignature()] = method; + methods[it.second.getSignature()] = method; } } doc["methods"] = methods; diff --git a/libsolidity/Types.cpp b/libsolidity/Types.cpp index d5b001636..c5f1a3ae5 100644 --- a/libsolidity/Types.cpp +++ b/libsolidity/Types.cpp @@ -489,7 +489,7 @@ MemberList const& ContractType::getMembers() const map> members(IntegerType::AddressMemberList.begin(), IntegerType::AddressMemberList.end()); for (auto const& it: m_contract.getInterfaceFunctions()) - members[it.second.second->getName()] = make_shared(*it.second.second, false); + members[it.second.getName()] = it.second.getFunctionTypeShared(); m_members.reset(new MemberList(members)); } return *m_members; @@ -511,9 +511,9 @@ shared_ptr const& ContractType::getConstructorType() const u256 ContractType::getFunctionIdentifier(string const& _functionName) const { auto interfaceFunctions = m_contract.getInterfaceFunctions(); - for (auto it = interfaceFunctions.cbegin(); it != interfaceFunctions.cend(); ++it) - if (it->second.second->getName() == _functionName) - return FixedHash<4>::Arith(it->first); + for (auto const& it: m_contract.getInterfaceFunctions()) + if (it.second.getName() == _functionName) + return FixedHash<4>::Arith(it.first); return Invalid256; } @@ -582,28 +582,47 @@ FunctionType::FunctionType(FunctionDefinition const& _function, bool _isInternal m_location(_isInternal ? Location::INTERNAL : Location::EXTERNAL) { TypePointers params; + vector paramNames; TypePointers retParams; + vector retParamNames; params.reserve(_function.getParameters().size()); + paramNames.reserve(_function.getParameters().size()); for (ASTPointer const& var: _function.getParameters()) + { + paramNames.push_back(var->getName()); params.push_back(var->getType()); + } retParams.reserve(_function.getReturnParameters().size()); + retParamNames.reserve(_function.getReturnParameters().size()); for (ASTPointer const& var: _function.getReturnParameters()) + { + retParamNames.push_back(var->getName()); retParams.push_back(var->getType()); + } swap(params, m_parameterTypes); + swap(paramNames, m_parameterNames); swap(retParams, m_returnParameterTypes); + swap(retParamNames, m_returnParameterNames); } FunctionType::FunctionType(VariableDeclaration const& _varDecl): m_location(Location::INTERNAL) { TypePointers params; + vector paramNames; TypePointers retParams; + vector retParamNames; // for now, no input parameters LTODO: change for some things like mapping params.reserve(0); + paramNames.reserve(0); retParams.reserve(1); + retParamNames.reserve(1); retParams.push_back(_varDecl.getType()); + retParamNames.push_back(_varDecl.getName()); swap(params, m_parameterTypes); + swap(paramNames, m_parameterNames); swap(retParams, m_returnParameterTypes); + swap(retParamNames, m_returnParameterNames); } bool FunctionType::operator==(Type const& _other) const diff --git a/libsolidity/Types.h b/libsolidity/Types.h index b3a92021b..b26157b3e 100644 --- a/libsolidity/Types.h +++ b/libsolidity/Types.h @@ -365,7 +365,9 @@ public: m_location(_location), m_gasSet(_gasSet), m_valueSet(_valueSet) {} TypePointers const& getParameterTypes() const { return m_parameterTypes; } + std::vector const& getParameterNames() const { return m_parameterNames; } TypePointers const& getReturnParameterTypes() const { return m_returnParameterTypes; } + std::vector const& getReturnParameterNames() const { return m_returnParameterNames; } virtual bool operator==(Type const& _other) const override; virtual std::string toString() const override; @@ -390,6 +392,8 @@ private: TypePointers m_parameterTypes; TypePointers m_returnParameterTypes; + std::vector m_parameterNames; + std::vector m_returnParameterNames; Location const m_location; bool const m_gasSet = false; ///< true iff the gas value to be used is on the stack bool const m_valueSet = false; ///< true iff the value to be sent is on the stack diff --git a/test/SolidityNameAndTypeResolution.cpp b/test/SolidityNameAndTypeResolution.cpp index 7728414de..dbb95cf72 100644 --- a/test/SolidityNameAndTypeResolution.cpp +++ b/test/SolidityNameAndTypeResolution.cpp @@ -30,6 +30,8 @@ #include #include +using namespace std; + namespace dev { namespace solidity @@ -39,6 +41,7 @@ namespace test namespace { + ASTPointer parseTextAndResolveNames(std::string const& _source) { Parser parser; @@ -90,8 +93,8 @@ static ContractDefinition const* retrieveContract(ASTPointer _source return NULL; } -static FunctionDefinition const* retrieveFunctionBySignature(ContractDefinition const* _contract, - std::string const& _signature) +static FunctionDescription const& retrieveFunctionBySignature(ContractDefinition const* _contract, + std::string const& _signature) { FixedHash<4> hash(dev::sha3(_signature)); return _contract->getInterfaceFunctions()[hash]; @@ -629,19 +632,40 @@ BOOST_AUTO_TEST_CASE(modifier_returns_value) BOOST_AUTO_TEST_CASE(state_variable_accessors) { - char const* text = "contract base {\n" + char const* text = "contract test {\n" + " function fun() {\n" + " uint64(2);\n" + " }\n" + "uint256 foo;\n" + "}\n"; + + ASTPointer source; + ContractDefinition const* contract; + BOOST_CHECK_NO_THROW(source = parseTextAndResolveNamesWithChecks(text)); + BOOST_CHECK((contract = retrieveContract(source, 0)) != nullptr); + FunctionDescription function = retrieveFunctionBySignature(contract, "foo()"); + BOOST_CHECK_MESSAGE(function.getDeclaration() != nullptr, "Could not find the accessor function"); + // vector const expected({ParamDescription("", "uint256")}); + // BOOST_CHECK_EQUAL_COLLECTIONS(function.getReturnParameters().begin(), function.getReturnParameters().end(), + // expected.begin(), expected.end()); +} + +BOOST_AUTO_TEST_CASE(private_state_variable) +{ + char const* text = "contract test {\n" " function fun() {\n" " uint64(2);\n" " }\n" + "private:\n" "uint256 foo;\n" "}\n"; ASTPointer source; ContractDefinition const* contract; - FunctionDefinition const* function; BOOST_CHECK_NO_THROW(source = parseTextAndResolveNamesWithChecks(text)); BOOST_CHECK((contract = retrieveContract(source, 0)) != nullptr); - BOOST_CHECK((function = retrieveFunctionBySignature(contract, "foo()")) != nullptr); + FunctionDescription function = retrieveFunctionBySignature(contract, "foo()"); + BOOST_CHECK_MESSAGE(function.getDeclaration() == nullptr, "Accessor function of a private variable should not exist"); } BOOST_AUTO_TEST_SUITE_END() From 3d1c0a9bef25bdbd8d54f4170d8b9c733c710813 Mon Sep 17 00:00:00 2001 From: Lefteris Karapetsas Date: Fri, 23 Jan 2015 17:55:58 +0100 Subject: [PATCH 06/18] Modifications to Mix to adapt to FunctionDescription --- libsolidity/AST.cpp | 10 ++++++++++ libsolidity/AST.h | 6 +++++- mix/QBasicNodeDefinition.h | 1 + mix/QContractDefinition.cpp | 7 ++----- mix/QFunctionDefinition.cpp | 31 ++++++++++++++++++++++++------- mix/QFunctionDefinition.h | 5 +---- mix/QVariableDeclaration.h | 1 + 7 files changed, 44 insertions(+), 17 deletions(-) diff --git a/libsolidity/AST.cpp b/libsolidity/AST.cpp index e4b5ed7d3..ca76d023c 100644 --- a/libsolidity/AST.cpp +++ b/libsolidity/AST.cpp @@ -607,6 +607,16 @@ Declaration const* FunctionDescription::getDeclaration() const return m_description.second; } +VariableDeclaration const* FunctionDescription::getVariableDeclaration() const +{ + return dynamic_cast(m_description.second); +} + +FunctionDefinition const* FunctionDescription::getFunctionDefinition() const +{ + return dynamic_cast(m_description.second); +} + shared_ptr FunctionDescription::getFunctionTypeShared() const { return m_description.first; diff --git a/libsolidity/AST.h b/libsolidity/AST.h index 89daf80fb..bda06a5db 100755 --- a/libsolidity/AST.h +++ b/libsolidity/AST.h @@ -200,8 +200,12 @@ struct FunctionDescription std::vector const getParameters() const; /// @returns the return parameters of the function std::vector const getReturnParameters() const; - /// @returns the Declaration AST Node pointer + /// @returns a generic Declaration AST Node pointer which can be either a FunctionDefinition or a VariableDeclaration Declaration const* getDeclaration() const; + /// @returns the VariableDeclaration AST Node pointer or nullptr if it's not a VariableDeclaration + VariableDeclaration const* getVariableDeclaration() const; + /// @returns the FunctionDefinition AST Node pointer or nullptr if it's not a FunctionDefinition + FunctionDefinition const* getFunctionDefinition() const; /// @returns a created shared pointer with the type of the function std::shared_ptr makeFunctionType() const; /// @returns a pointer to the function type diff --git a/mix/QBasicNodeDefinition.h b/mix/QBasicNodeDefinition.h index 0d7365b72..d276d0eb2 100644 --- a/mix/QBasicNodeDefinition.h +++ b/mix/QBasicNodeDefinition.h @@ -38,6 +38,7 @@ public: QBasicNodeDefinition(): QObject() {} ~QBasicNodeDefinition() {} QBasicNodeDefinition(solidity::Declaration const* _d): QObject(), m_name(QString::fromStdString(_d->getName())) {} + QBasicNodeDefinition(std::string const& _name): QObject(), m_name(QString::fromStdString(_name)) {} /// Get the name of the node. QString name() const { return m_name; } diff --git a/mix/QContractDefinition.cpp b/mix/QContractDefinition.cpp index 9f6597d83..bcd4b4711 100644 --- a/mix/QContractDefinition.cpp +++ b/mix/QContractDefinition.cpp @@ -38,9 +38,6 @@ QContractDefinition::QContractDefinition(dev::solidity::ContractDefinition const else m_constructor = new QFunctionDefinition(); - auto interfaceFunctions = _contract->getInterfaceFunctions(); - unsigned i = 0; - for (auto it = interfaceFunctions.cbegin(); it != interfaceFunctions.cend(); ++it, ++i) - m_functions.append(new QFunctionDefinition(it->second, i)); -} + for (auto const& it: _contract->getInterfaceFunctions()) + m_functions.append(new QFunctionDefinition(it.second));} diff --git a/mix/QFunctionDefinition.cpp b/mix/QFunctionDefinition.cpp index 97ce0ff58..3c1e800ca 100644 --- a/mix/QFunctionDefinition.cpp +++ b/mix/QFunctionDefinition.cpp @@ -21,19 +21,36 @@ #include #include +#include #include "QVariableDeclaration.h" #include "QFunctionDefinition.h" using namespace dev::solidity; using namespace dev::mix; -QFunctionDefinition::QFunctionDefinition(dev::solidity::FunctionDefinition const* _f, int _index): QBasicNodeDefinition(_f), m_index(_index), m_hash(dev::sha3(_f->getCanonicalSignature())) +QFunctionDefinition::QFunctionDefinition(dev::solidity::FunctionDescription const& _f): QBasicNodeDefinition(_f.getDeclaration()), m_hash(dev::sha3(_f.getSignature())) { - std::vector> parameters = _f->getParameterList().getParameters(); - for (unsigned i = 0; i < parameters.size(); i++) - m_parameters.append(new QVariableDeclaration(parameters.at(i).get())); + FunctionDefinition const* funcDef; + VariableDeclaration const* varDecl; + if ((funcDef = _f.getFunctionDefinition())) + { + std::vector> parameters = funcDef->getParameterList().getParameters(); + for (unsigned i = 0; i < parameters.size(); i++) + m_parameters.append(new QVariableDeclaration(parameters.at(i).get())); - std::vector> returnParameters = _f->getReturnParameters(); - for (unsigned i = 0; i < returnParameters.size(); i++) - m_returnParameters.append(new QVariableDeclaration(returnParameters.at(i).get())); + std::vector> returnParameters = funcDef->getReturnParameters(); + for (unsigned i = 0; i < returnParameters.size(); i++) + m_returnParameters.append(new QVariableDeclaration(returnParameters.at(i).get())); + } + else + { + if (!(varDecl = _f.getVariableDeclaration())) + BOOST_THROW_EXCEPTION(Exception() << errinfo_comment("Malformed FunctionDescription. Should never happen.")); + + // only the return parameter for now. + // TODO: change this for other state variables like mapping and maybe abstract this inside solidity and not here + auto returnParams = _f.getReturnParameters(); + m_returnParameters.append(new QVariableDeclaration(returnParams[0])); + + } } diff --git a/mix/QFunctionDefinition.h b/mix/QFunctionDefinition.h index 7f606c8a1..2b7084206 100644 --- a/mix/QFunctionDefinition.h +++ b/mix/QFunctionDefinition.h @@ -36,19 +36,16 @@ class QFunctionDefinition: public QBasicNodeDefinition { Q_OBJECT Q_PROPERTY(QQmlListProperty parameters READ parameters) - Q_PROPERTY(int index READ index) public: QFunctionDefinition() {} - QFunctionDefinition(solidity::FunctionDefinition const* _f, int _index); + QFunctionDefinition(solidity::FunctionDescription const& _f); /// Get all input parameters of this function. QList const& parametersList() const { return m_parameters; } /// Get all input parameters of this function as QML property. QQmlListProperty parameters() const { return QQmlListProperty(const_cast(this), const_cast(this)->m_parameters); } /// Get all return parameters of this function. QList returnParameters() const { return m_returnParameters; } - /// Get the index of this function on the contract ABI. - int index() const { return m_index; } /// Get the hash of this function declaration on the contract ABI. FixedHash<4> hash() const { return m_hash; } diff --git a/mix/QVariableDeclaration.h b/mix/QVariableDeclaration.h index 966ee0ff3..ff4018fa3 100644 --- a/mix/QVariableDeclaration.h +++ b/mix/QVariableDeclaration.h @@ -37,6 +37,7 @@ class QVariableDeclaration: public QBasicNodeDefinition public: QVariableDeclaration() {} QVariableDeclaration(solidity::VariableDeclaration const* _v): QBasicNodeDefinition(_v), m_type(QString::fromStdString(_v->getType()->toString())) {} + QVariableDeclaration(solidity::ParamDescription const& _v): QBasicNodeDefinition(_v.getName()), m_type(QString::fromStdString(_v.getType())) {} QString type() const { return m_type; } private: QString m_type; From 3732d42ce885382523d211be6444f81911c89dcb Mon Sep 17 00:00:00 2001 From: Lefteris Karapetsas Date: Mon, 26 Jan 2015 09:48:29 +0100 Subject: [PATCH 07/18] Various small fixes for Sol Automatic Accessors --- libsolidity/AST.cpp | 14 +------------- libsolidity/AST.h | 3 --- libsolidity/Types.cpp | 2 +- libsolidity/Types.h | 2 +- test/SolidityNameAndTypeResolution.cpp | 6 +++--- test/SolidityParser.cpp | 2 +- 6 files changed, 7 insertions(+), 22 deletions(-) diff --git a/libsolidity/AST.cpp b/libsolidity/AST.cpp index ca76d023c..38adebf9d 100644 --- a/libsolidity/AST.cpp +++ b/libsolidity/AST.cpp @@ -519,17 +519,6 @@ void Literal::checkTypeRequirements() BOOST_THROW_EXCEPTION(createTypeError("Invalid literal value.")); } - -bool ParamDescription::operator!=(ParamDescription const& _other) const -{ - return m_description.first == _other.getName() && m_description.second == _other.getType(); -} - -std::ostream& ParamDescription::operator<<(std::ostream& os) const -{ - return os << m_description.first << ":" << m_description.second; -} - std::string ParamDescription::getName() const { return m_description.first; @@ -540,7 +529,6 @@ std::string ParamDescription::getType() const return m_description.second; } - ASTPointer FunctionDescription::getDocumentation() const { auto function = dynamic_cast(m_description.second); @@ -575,7 +563,7 @@ vector const FunctionDescription::getParameters() const if (function) { vector paramsDescription; - for (auto const& param: function->getParameters()) + for (auto const& param: function->getReturnParameters()) paramsDescription.push_back(ParamDescription(param->getName(), param->getType()->toString())); return paramsDescription; diff --git a/libsolidity/AST.h b/libsolidity/AST.h index bda06a5db..d9a95e506 100755 --- a/libsolidity/AST.h +++ b/libsolidity/AST.h @@ -166,9 +166,6 @@ struct ParamDescription ParamDescription(std::string const& _name, std::string const& _type): m_description(_name, _type){} - bool operator!=(ParamDescription const& _other) const; - std::ostream& operator<<(std::ostream& os) const; - std::string getName() const; std::string getType() const; diff --git a/libsolidity/Types.cpp b/libsolidity/Types.cpp index c5f1a3ae5..812271e30 100644 --- a/libsolidity/Types.cpp +++ b/libsolidity/Types.cpp @@ -606,7 +606,7 @@ FunctionType::FunctionType(FunctionDefinition const& _function, bool _isInternal } FunctionType::FunctionType(VariableDeclaration const& _varDecl): - m_location(Location::INTERNAL) + m_location(Location::EXTERNAL) { TypePointers params; vector paramNames; diff --git a/libsolidity/Types.h b/libsolidity/Types.h index b26157b3e..2dbed95fa 100644 --- a/libsolidity/Types.h +++ b/libsolidity/Types.h @@ -378,7 +378,7 @@ public: virtual MemberList const& getMembers() const override; Location const& getLocation() const { return m_location; } - std::string getCanonicalSignature(std::string const &_name) const; + std::string getCanonicalSignature(std::string const& _name) const; bool gasSet() const { return m_gasSet; } bool valueSet() const { return m_valueSet; } diff --git a/test/SolidityNameAndTypeResolution.cpp b/test/SolidityNameAndTypeResolution.cpp index dbb95cf72..3b711bfec 100644 --- a/test/SolidityNameAndTypeResolution.cpp +++ b/test/SolidityNameAndTypeResolution.cpp @@ -645,9 +645,9 @@ BOOST_AUTO_TEST_CASE(state_variable_accessors) BOOST_CHECK((contract = retrieveContract(source, 0)) != nullptr); FunctionDescription function = retrieveFunctionBySignature(contract, "foo()"); BOOST_CHECK_MESSAGE(function.getDeclaration() != nullptr, "Could not find the accessor function"); - // vector const expected({ParamDescription("", "uint256")}); - // BOOST_CHECK_EQUAL_COLLECTIONS(function.getReturnParameters().begin(), function.getReturnParameters().end(), - // expected.begin(), expected.end()); + auto returnParams = function.getReturnParameters(); + BOOST_CHECK_EQUAL(returnParams.at(0).getType(), "uint256"); + BOOST_CHECK(function.isConstant()); } BOOST_AUTO_TEST_CASE(private_state_variable) diff --git a/test/SolidityParser.cpp b/test/SolidityParser.cpp index db7806f4d..7bfb4c0c8 100644 --- a/test/SolidityParser.cpp +++ b/test/SolidityParser.cpp @@ -204,7 +204,7 @@ BOOST_AUTO_TEST_CASE(multiline_function_documentation) BOOST_REQUIRE_NO_THROW(contract = parseText(text)); auto functions = contract->getDefinedFunctions(); - BOOST_REQUIRE_NO_THROW(function = functions.at(1)); // 1 since, 0 is the index of stateVar accessor + BOOST_REQUIRE_NO_THROW(function = functions.at(0)); checkFunctionNatspec(function, "This is a test function\n" " and it has 2 lines"); } From 10da71f90e1eda67114b5e6eeed1795bda6573ab Mon Sep 17 00:00:00 2001 From: Lefteris Karapetsas Date: Mon, 26 Jan 2015 14:41:56 +0100 Subject: [PATCH 08/18] Solidity EntryLabel now uses a generic declaration - Instead of a FunctionDefinition --- libsolidity/Compiler.cpp | 10 +++++++--- libsolidity/CompilerContext.cpp | 6 +++--- libsolidity/CompilerContext.h | 2 +- 3 files changed, 11 insertions(+), 7 deletions(-) diff --git a/libsolidity/Compiler.cpp b/libsolidity/Compiler.cpp index f6f48a8c2..28a9b3d10 100644 --- a/libsolidity/Compiler.cpp +++ b/libsolidity/Compiler.cpp @@ -43,9 +43,13 @@ void Compiler::compileContract(ContractDefinition const& _contract, for (ContractDefinition const* contract: _contract.getLinearizedBaseContracts()) { - for (ASTPointer const& function: contract->getDefinedFunctions()) - if (!function->isConstructor()) - m_context.addFunction(*function); + for (auto const& it: contract->getInterfaceFunctions()) + { + auto funcDef = it.second.getFunctionDefinition(); + if (funcDef && funcDef->isConstructor()) + continue; + m_context.addFunction(*it.second.getDeclaration()); + } for (ASTPointer const& modifier: contract->getFunctionModifiers()) m_context.addModifier(*modifier); } diff --git a/libsolidity/CompilerContext.cpp b/libsolidity/CompilerContext.cpp index 4edced940..ea349c0d2 100644 --- a/libsolidity/CompilerContext.cpp +++ b/libsolidity/CompilerContext.cpp @@ -59,11 +59,11 @@ void CompilerContext::addAndInitializeVariable(VariableDeclaration const& _decla *this << u256(0); } -void CompilerContext::addFunction(FunctionDefinition const& _function) +void CompilerContext::addFunction(Declaration const& _decl) { eth::AssemblyItem tag(m_asm.newTag()); - m_functionEntryLabels.insert(make_pair(&_function, tag)); - m_virtualFunctionEntryLabels.insert(make_pair(_function.getName(), tag)); + m_functionEntryLabels.insert(make_pair(&_decl, tag)); + m_virtualFunctionEntryLabels.insert(make_pair(_decl.getName(), tag)); } void CompilerContext::addModifier(ModifierDefinition const& _modifier) diff --git a/libsolidity/CompilerContext.h b/libsolidity/CompilerContext.h index aa438cf02..42ac9ee82 100644 --- a/libsolidity/CompilerContext.h +++ b/libsolidity/CompilerContext.h @@ -44,7 +44,7 @@ public: void startNewFunction() { m_localVariables.clear(); m_asm.setDeposit(0); } void addVariable(VariableDeclaration const& _declaration, unsigned _offsetToCurrent = 0); void addAndInitializeVariable(VariableDeclaration const& _declaration); - void addFunction(FunctionDefinition const& _function); + void addFunction(Declaration const& _decl); /// Adds the given modifier to the list by name if the name is not present already. void addModifier(ModifierDefinition const& _modifier); From 37eb587dd9be0dbc367acab59a9b32347d6061ab Mon Sep 17 00:00:00 2001 From: Lefteris Karapetsas Date: Mon, 26 Jan 2015 16:14:40 +0100 Subject: [PATCH 09/18] Adding isStateVariable attribute to a VarDecl --- libsolidity/AST.h | 6 ++++-- libsolidity/Parser.cpp | 6 +++--- libsolidity/Parser.h | 2 +- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/libsolidity/AST.h b/libsolidity/AST.h index d9a95e506..d769aa225 100755 --- a/libsolidity/AST.h +++ b/libsolidity/AST.h @@ -430,8 +430,8 @@ class VariableDeclaration: public Declaration { public: VariableDeclaration(Location const& _location, ASTPointer const& _type, - ASTPointer const& _name, bool _isPublic): - Declaration(_location, _name), m_typeName(_type), m_isPublic(_isPublic) {} + ASTPointer const& _name, bool _isPublic, bool _isStateVar = false): + Declaration(_location, _name), m_typeName(_type), m_isPublic(_isPublic), m_isStateVariable(_isStateVar) {} virtual void accept(ASTVisitor& _visitor) override; virtual void accept(ASTConstVisitor& _visitor) const override; @@ -445,11 +445,13 @@ public: virtual LValueType getLValueType() const override; bool isLocalVariable() const { return !!dynamic_cast(getScope()); } bool isPublic() const { return m_isPublic; } + bool isStateVariable() const { return m_isStateVariable; } private: ASTPointer m_typeName; ///< can be empty ("var") bool m_isPublic; ///< Whether there is an accessor for it or not + bool m_isStateVariable; ///< Whether or not this is a contract state variable std::shared_ptr m_type; ///< derived type, initially empty }; diff --git a/libsolidity/Parser.cpp b/libsolidity/Parser.cpp index 1c61aab12..5cfc8f462 100644 --- a/libsolidity/Parser.cpp +++ b/libsolidity/Parser.cpp @@ -150,7 +150,7 @@ ASTPointer Parser::parseContractDefinition() Token::isElementaryTypeName(currentToken)) { bool const allowVar = false; - stateVariables.push_back(parseVariableDeclaration(allowVar, visibilityIsPublic)); + stateVariables.push_back(parseVariableDeclaration(allowVar, visibilityIsPublic, true)); expectToken(Token::SEMICOLON); } else if (currentToken == Token::MODIFIER) @@ -245,12 +245,12 @@ ASTPointer Parser::parseStructDefinition() return nodeFactory.createNode(name, members); } -ASTPointer Parser::parseVariableDeclaration(bool _allowVar, bool _isPublic) +ASTPointer Parser::parseVariableDeclaration(bool _allowVar, bool _isPublic, bool _isStateVariable) { ASTNodeFactory nodeFactory(*this); ASTPointer type = parseTypeName(_allowVar); nodeFactory.markEndPosition(); - return nodeFactory.createNode(type, expectIdentifierToken(), _isPublic); + return nodeFactory.createNode(type, expectIdentifierToken(), _isPublic, _isStateVariable); } ASTPointer Parser::parseModifierDefinition() diff --git a/libsolidity/Parser.h b/libsolidity/Parser.h index a8f97c6e3..d3bff67e5 100644 --- a/libsolidity/Parser.h +++ b/libsolidity/Parser.h @@ -52,7 +52,7 @@ private: ASTPointer parseInheritanceSpecifier(); ASTPointer parseFunctionDefinition(bool _isPublic, ASTString const* _contractName); ASTPointer parseStructDefinition(); - ASTPointer parseVariableDeclaration(bool _allowVar, bool _isPublic = false); + ASTPointer parseVariableDeclaration(bool _allowVar, bool _isPublic = false, bool _isStateVar = false); ASTPointer parseModifierDefinition(); ASTPointer parseModifierInvocation(); ASTPointer parseIdentifier(); From 94ca9f0040fe9abe89ab8f9ba6b7efcdc04c1eba Mon Sep 17 00:00:00 2001 From: Lefteris Karapetsas Date: Mon, 26 Jan 2015 16:58:54 +0100 Subject: [PATCH 10/18] All interface functions are external. --- libsolidity/AST.cpp | 2 +- libsolidity/Compiler.cpp | 40 ++++++++++++++++++++++++++++++++-------- libsolidity/Compiler.h | 2 ++ 3 files changed, 35 insertions(+), 9 deletions(-) diff --git a/libsolidity/AST.cpp b/libsolidity/AST.cpp index 38adebf9d..bf3d98947 100644 --- a/libsolidity/AST.cpp +++ b/libsolidity/AST.cpp @@ -151,7 +151,7 @@ vector, std::shared_ptr, Declaration cons { functionsSeen.insert(f->getName()); FixedHash<4> hash(dev::sha3(f->getCanonicalSignature())); - m_interfaceFunctionList->push_back(make_tuple(hash, make_shared(*f), f.get())); + m_interfaceFunctionList->push_back(make_tuple(hash, make_shared(*f, false), f.get())); } for (ASTPointer const& v: contract->getStateVariables()) diff --git a/libsolidity/Compiler.cpp b/libsolidity/Compiler.cpp index 28a9b3d10..c419f484d 100644 --- a/libsolidity/Compiler.cpp +++ b/libsolidity/Compiler.cpp @@ -43,23 +43,30 @@ void Compiler::compileContract(ContractDefinition const& _contract, for (ContractDefinition const* contract: _contract.getLinearizedBaseContracts()) { - for (auto const& it: contract->getInterfaceFunctions()) - { - auto funcDef = it.second.getFunctionDefinition(); - if (funcDef && funcDef->isConstructor()) - continue; - m_context.addFunction(*it.second.getDeclaration()); - } - for (ASTPointer const& modifier: contract->getFunctionModifiers()) + for (ASTPointer const& function: contract->getDefinedFunctions()) + if (!function->isConstructor()) + m_context.addFunction(*function); + + for (ASTPointer const& vardecl: contract->getStateVariables()) + if (vardecl->isPublic()) + m_context.addFunction(*vardecl); + + for (ASTPointer const& modifier: contract->getFunctionModifiers()) m_context.addModifier(*modifier); } appendFunctionSelector(_contract); for (ContractDefinition const* contract: _contract.getLinearizedBaseContracts()) + { for (ASTPointer const& function: contract->getDefinedFunctions()) if (!function->isConstructor()) function->accept(*this); + for (ASTPointer const& vardecl: contract->getStateVariables()) + if (vardecl->isPublic()) + generateAccessorCode(*vardecl); + } + // Swap the runtime context with the creation-time context swap(m_context, m_runtimeContext); initializeContext(_contract, _contracts); @@ -285,6 +292,23 @@ void Compiler::registerStateVariables(ContractDefinition const& _contract) m_context.addStateVariable(*variable); } +bool Compiler::generateAccessorCode(VariableDeclaration const& _varDecl) +{ + m_context.startNewFunction(); + m_returnTag = m_context.newTag(); + m_breakTags.clear(); + m_continueTags.clear(); + + // TODO: Work in progress + m_context << m_context.getFunctionEntryLabel(_varDecl); + // CompilerUtils(m_context).moveToStackVariable(firstVariable); + m_context.appendJumpTo(m_returnTag); + m_context << m_returnTag; + + // TODO: perhaps return void if there are no checks? + return true; +} + bool Compiler::visit(FunctionDefinition const& _function) { //@todo to simplify this, the calling convention could by changed such that diff --git a/libsolidity/Compiler.h b/libsolidity/Compiler.h index f40339b03..71a12a667 100644 --- a/libsolidity/Compiler.h +++ b/libsolidity/Compiler.h @@ -63,6 +63,8 @@ private: void registerStateVariables(ContractDefinition const& _contract); + bool generateAccessorCode(VariableDeclaration const& _varDecl); + virtual bool visit(FunctionDefinition const& _function) override; virtual bool visit(IfStatement const& _ifStatement) override; virtual bool visit(WhileStatement const& _whileStatement) override; From 85e4b2926093a47dc3870b4affe20eb92a01feb0 Mon Sep 17 00:00:00 2001 From: Lefteris Karapetsas Date: Mon, 26 Jan 2015 18:16:47 +0100 Subject: [PATCH 11/18] Work on ExpressionCompiler preparing for Accessors from storage --- libsolidity/ExpressionCompiler.cpp | 55 +++++++++++++++++++----------- libsolidity/ExpressionCompiler.h | 8 +++++ 2 files changed, 44 insertions(+), 19 deletions(-) diff --git a/libsolidity/ExpressionCompiler.cpp b/libsolidity/ExpressionCompiler.cpp index bd8c86531..450cc2fec 100644 --- a/libsolidity/ExpressionCompiler.cpp +++ b/libsolidity/ExpressionCompiler.cpp @@ -789,6 +789,13 @@ unsigned ExpressionCompiler::appendArgumentCopyToMemory(TypePointers const& _typ return length; } +void ExpressionCompiler::appendStateVariableAccessor(VariableDeclaration const* _varDecl) +{ + m_currentLValue.fromStateVariable(*_varDecl, _varDecl->getType()); + // TODO + // m_currentLValue.retrieveValueFromStorage(); +} + ExpressionCompiler::LValue::LValue(CompilerContext& _compilerContext, LValueType _type, Type const& _dataType, unsigned _baseStackOffset): m_context(&_compilerContext), m_type(_type), m_baseStackOffset(_baseStackOffset) @@ -816,21 +823,7 @@ void ExpressionCompiler::LValue::retrieveValue(Expression const& _expression, bo break; } case STORAGE: - if (!_expression.getType()->isValueType()) - break; // no distinction between value and reference for non-value types - if (!_remove) - *m_context << eth::Instruction::DUP1; - if (m_size == 1) - *m_context << eth::Instruction::SLOAD; - else - for (unsigned i = 0; i < m_size; ++i) - { - *m_context << eth::Instruction::DUP1 << eth::Instruction::SLOAD << eth::Instruction::SWAP1; - if (i + 1 < m_size) - *m_context << u256(1) << eth::Instruction::ADD; - else - *m_context << eth::Instruction::POP; - } + retrieveValueFromStorage(_expression, _remove); break; case MEMORY: if (!_expression.getType()->isValueType()) @@ -845,6 +838,25 @@ void ExpressionCompiler::LValue::retrieveValue(Expression const& _expression, bo } } +void ExpressionCompiler::LValue::retrieveValueFromStorage(Expression const& _expression, bool _remove) const +{ + if (!_expression.getType()->isValueType()) + return; // no distinction between value and reference for non-value types + if (!_remove) + *m_context << eth::Instruction::DUP1; + if (m_size == 1) + *m_context << eth::Instruction::SLOAD; + else + for (unsigned i = 0; i < m_size; ++i) + { + *m_context << eth::Instruction::DUP1 << eth::Instruction::SLOAD << eth::Instruction::SWAP1; + if (i + 1 < m_size) + *m_context << u256(1) << eth::Instruction::ADD; + else + *m_context << eth::Instruction::POP; + } +} + void ExpressionCompiler::LValue::storeValue(Expression const& _expression, bool _move) const { switch (m_type) @@ -951,6 +963,14 @@ void ExpressionCompiler::LValue::retrieveValueIfLValueNotRequested(Expression co } } +void ExpressionCompiler::LValue::fromStateVariable(Declaration const& _varDecl, std::shared_ptr const& _type) +{ + m_type = STORAGE; + solAssert(_type->getStorageSize() <= numeric_limits::max(), "The storage size of " + _type->toString() + " should fit in an unsigned"); + *m_context << m_context->getStorageLocationOfVariable(_varDecl); + m_size = unsigned(_type->getStorageSize()); +} + void ExpressionCompiler::LValue::fromIdentifier(Identifier const& _identifier, Declaration const& _declaration) { if (m_context->isLocalVariable(&_declaration)) @@ -961,10 +981,7 @@ void ExpressionCompiler::LValue::fromIdentifier(Identifier const& _identifier, D } else if (m_context->isStateVariable(&_declaration)) { - m_type = STORAGE; - solAssert(_identifier.getType()->getStorageSize() <= numeric_limits::max(), "The storage size of " + _identifier.getType()->toString() + " should fit in unsigned"); - m_size = unsigned(_identifier.getType()->getStorageSize()); - *m_context << m_context->getStorageLocationOfVariable(_declaration); + fromStateVariable(_declaration, _identifier.getType()); } else BOOST_THROW_EXCEPTION(InternalCompilerError() << errinfo_sourceLocation(_identifier.getLocation()) diff --git a/libsolidity/ExpressionCompiler.h b/libsolidity/ExpressionCompiler.h index 8f784fde9..7189bef99 100644 --- a/libsolidity/ExpressionCompiler.h +++ b/libsolidity/ExpressionCompiler.h @@ -95,6 +95,9 @@ private: unsigned appendArgumentCopyToMemory(TypePointers const& _functionType, std::vector> const& _arguments, unsigned _memoryOffset = 0); + /// Appends code for a State Variable accessor function + void appendStateVariableAccessor(VariableDeclaration const* _varDecl); + /** * Helper class to store and retrieve lvalues to and from various locations. * All types except STACK store a reference in a slot on the stack, STACK just @@ -111,6 +114,8 @@ private: /// Set type according to the declaration and retrieve the reference. /// @a _expression is the current expression void fromIdentifier(Identifier const& _identifier, Declaration const& _declaration); + /// Convenience function to set type for a state variable and retrieve the reference + void fromStateVariable(Declaration const& _varDecl, std::shared_ptr const& _type); void reset() { m_type = NONE; m_baseStackOffset = 0; m_size = 0; } bool isValid() const { return m_type != NONE; } @@ -125,6 +130,9 @@ private: /// also removes the reference from the stack (note that is does not reset the type to @a NONE). /// @a _expression is the current expression, used for error reporting. void retrieveValue(Expression const& _expression, bool _remove = false) const; + /// Convenience function to retrive Value from Storage. Specific version of + /// @ref retrieveValue + void retrieveValueFromStorage(Expression const& _expression, bool _remove = false) const; /// Stores a value (from the stack directly beneath the reference, which is assumed to /// be on the top of the stack, if any) in the lvalue and removes the reference. /// Also removes the stored value from the stack if @a _move is From 50a4b6055bb6401836e2e18ee38c29f06e5acc17 Mon Sep 17 00:00:00 2001 From: Lefteris Karapetsas Date: Tue, 27 Jan 2015 15:43:28 +0100 Subject: [PATCH 12/18] Fixes after rebasing on develop --- libsolidity/AST.cpp | 4 ++-- libsolidity/AST.h | 4 ++++ libsolidity/ExpressionCompiler.h | 3 +-- mix/QContractDefinition.cpp | 5 ++++- 4 files changed, 11 insertions(+), 5 deletions(-) diff --git a/libsolidity/AST.cpp b/libsolidity/AST.cpp index bf3d98947..71bf7c4ab 100644 --- a/libsolidity/AST.cpp +++ b/libsolidity/AST.cpp @@ -563,7 +563,7 @@ vector const FunctionDescription::getParameters() const if (function) { vector paramsDescription; - for (auto const& param: function->getReturnParameters()) + for (auto const& param: function->getParameters()) paramsDescription.push_back(ParamDescription(param->getName(), param->getType()->toString())); return paramsDescription; @@ -580,7 +580,7 @@ vector const FunctionDescription::getReturnParameters() const if (function) { vector paramsDescription; - for (auto const& param: function->getParameters()) + for (auto const& param: function->getReturnParameters()) paramsDescription.push_back(ParamDescription(param->getName(), param->getType()->toString())); return paramsDescription; diff --git a/libsolidity/AST.h b/libsolidity/AST.h index d769aa225..a99bc39cb 100755 --- a/libsolidity/AST.h +++ b/libsolidity/AST.h @@ -182,6 +182,10 @@ struct FunctionDescription FunctionDescription(std::shared_ptr _type, Declaration const* _decl): m_description(_type, _decl){} + /// constructor for a constructor's function definition. Used only inside mix. + FunctionDescription(Declaration const* _def): + m_description(nullptr, _def){} + FunctionDescription(): m_description(nullptr, nullptr){} diff --git a/libsolidity/ExpressionCompiler.h b/libsolidity/ExpressionCompiler.h index 7189bef99..bff2cd7ce 100644 --- a/libsolidity/ExpressionCompiler.h +++ b/libsolidity/ExpressionCompiler.h @@ -130,8 +130,7 @@ private: /// also removes the reference from the stack (note that is does not reset the type to @a NONE). /// @a _expression is the current expression, used for error reporting. void retrieveValue(Expression const& _expression, bool _remove = false) const; - /// Convenience function to retrive Value from Storage. Specific version of - /// @ref retrieveValue + /// Convenience function to retrive Value from Storage. Specific version of @ref retrieveValue void retrieveValueFromStorage(Expression const& _expression, bool _remove = false) const; /// Stores a value (from the stack directly beneath the reference, which is assumed to /// be on the top of the stack, if any) in the lvalue and removes the reference. diff --git a/mix/QContractDefinition.cpp b/mix/QContractDefinition.cpp index bcd4b4711..503f93c39 100644 --- a/mix/QContractDefinition.cpp +++ b/mix/QContractDefinition.cpp @@ -34,7 +34,10 @@ using namespace dev::mix; QContractDefinition::QContractDefinition(dev::solidity::ContractDefinition const* _contract): QBasicNodeDefinition(_contract) { if (_contract->getConstructor() != nullptr) - m_constructor = new QFunctionDefinition(_contract->getConstructor(), -1); + { + FunctionDescription desc(_contract->getConstructor()); + m_constructor = new QFunctionDefinition(desc); + } else m_constructor = new QFunctionDefinition(); From 309ffb89483b8e738b8b69720030f5c201abd88a Mon Sep 17 00:00:00 2001 From: Lefteris Karapetsas Date: Tue, 27 Jan 2015 16:55:06 +0100 Subject: [PATCH 13/18] EVM Code for simple accessor function is properly generated --- libsolidity/Compiler.cpp | 12 +++++------- libsolidity/Compiler.h | 2 +- libsolidity/ExpressionCompiler.cpp | 15 ++++++++++----- libsolidity/ExpressionCompiler.h | 6 ++++-- test/SolidityEndToEndTest.cpp | 12 ++++++++++++ 5 files changed, 32 insertions(+), 15 deletions(-) diff --git a/libsolidity/Compiler.cpp b/libsolidity/Compiler.cpp index c419f484d..bda18bc68 100644 --- a/libsolidity/Compiler.cpp +++ b/libsolidity/Compiler.cpp @@ -292,21 +292,19 @@ void Compiler::registerStateVariables(ContractDefinition const& _contract) m_context.addStateVariable(*variable); } -bool Compiler::generateAccessorCode(VariableDeclaration const& _varDecl) +void Compiler::generateAccessorCode(VariableDeclaration const& _varDecl) { m_context.startNewFunction(); m_returnTag = m_context.newTag(); m_breakTags.clear(); m_continueTags.clear(); - // TODO: Work in progress m_context << m_context.getFunctionEntryLabel(_varDecl); - // CompilerUtils(m_context).moveToStackVariable(firstVariable); - m_context.appendJumpTo(m_returnTag); - m_context << m_returnTag; + ExpressionCompiler::appendStateVariableAccessor(m_context, &_varDecl); - // TODO: perhaps return void if there are no checks? - return true; + uint64_t foo = uint64_t(_varDecl.getType()->getStorageSize()); + m_context << eth::dupInstruction(foo + 1); + m_context << eth::Instruction::JUMP; } bool Compiler::visit(FunctionDefinition const& _function) diff --git a/libsolidity/Compiler.h b/libsolidity/Compiler.h index 71a12a667..144af8eb8 100644 --- a/libsolidity/Compiler.h +++ b/libsolidity/Compiler.h @@ -63,7 +63,7 @@ private: void registerStateVariables(ContractDefinition const& _contract); - bool generateAccessorCode(VariableDeclaration const& _varDecl); + void generateAccessorCode(VariableDeclaration const& _varDecl); virtual bool visit(FunctionDefinition const& _function) override; virtual bool visit(IfStatement const& _ifStatement) override; diff --git a/libsolidity/ExpressionCompiler.cpp b/libsolidity/ExpressionCompiler.cpp index 450cc2fec..66e2c68cc 100644 --- a/libsolidity/ExpressionCompiler.cpp +++ b/libsolidity/ExpressionCompiler.cpp @@ -48,6 +48,12 @@ void ExpressionCompiler::appendTypeConversion(CompilerContext& _context, Type co compiler.appendTypeConversion(_typeOnStack, _targetType, _cleanupNeeded); } +void ExpressionCompiler::appendStateVariableAccessor(CompilerContext& _context, VariableDeclaration const* _varDecl, bool _optimize) +{ + ExpressionCompiler compiler(_context, _optimize); + compiler.appendStateVariableAccessor(_varDecl); +} + bool ExpressionCompiler::visit(Assignment const& _assignment) { _assignment.getRightHandSide().accept(*this); @@ -792,8 +798,7 @@ unsigned ExpressionCompiler::appendArgumentCopyToMemory(TypePointers const& _typ void ExpressionCompiler::appendStateVariableAccessor(VariableDeclaration const* _varDecl) { m_currentLValue.fromStateVariable(*_varDecl, _varDecl->getType()); - // TODO - // m_currentLValue.retrieveValueFromStorage(); + m_currentLValue.retrieveValueFromStorage(_varDecl->getType(), true); } ExpressionCompiler::LValue::LValue(CompilerContext& _compilerContext, LValueType _type, Type const& _dataType, @@ -823,7 +828,7 @@ void ExpressionCompiler::LValue::retrieveValue(Expression const& _expression, bo break; } case STORAGE: - retrieveValueFromStorage(_expression, _remove); + retrieveValueFromStorage(_expression.getType(), _remove); break; case MEMORY: if (!_expression.getType()->isValueType()) @@ -838,9 +843,9 @@ void ExpressionCompiler::LValue::retrieveValue(Expression const& _expression, bo } } -void ExpressionCompiler::LValue::retrieveValueFromStorage(Expression const& _expression, bool _remove) const +void ExpressionCompiler::LValue::retrieveValueFromStorage(std::shared_ptr const& _type, bool _remove) const { - if (!_expression.getType()->isValueType()) + if (!_type->isValueType()) return; // no distinction between value and reference for non-value types if (!_remove) *m_context << eth::Instruction::DUP1; diff --git a/libsolidity/ExpressionCompiler.h b/libsolidity/ExpressionCompiler.h index bff2cd7ce..8479344ad 100644 --- a/libsolidity/ExpressionCompiler.h +++ b/libsolidity/ExpressionCompiler.h @@ -53,6 +53,8 @@ public: /// Appends code to remove dirty higher order bits in case of an implicit promotion to a wider type. static void appendTypeConversion(CompilerContext& _context, Type const& _typeOnStack, Type const& _targetType, bool _cleanupNeeded = false); + /// Appends code for a State Variable accessor function + static void appendStateVariableAccessor(CompilerContext& _context, VariableDeclaration const* _varDecl, bool _optimize = false); private: explicit ExpressionCompiler(CompilerContext& _compilerContext, bool _optimize = false): @@ -130,8 +132,8 @@ private: /// also removes the reference from the stack (note that is does not reset the type to @a NONE). /// @a _expression is the current expression, used for error reporting. void retrieveValue(Expression const& _expression, bool _remove = false) const; - /// Convenience function to retrive Value from Storage. Specific version of @ref retrieveValue - void retrieveValueFromStorage(Expression const& _expression, bool _remove = false) const; + /// Convenience function to retrieve Value from Storage. Specific version of @ref retrieveValue + void retrieveValueFromStorage(std::shared_ptr const& _type, bool _remove = false) const; /// Stores a value (from the stack directly beneath the reference, which is assumed to /// be on the top of the stack, if any) in the lvalue and removes the reference. /// Also removes the stored value from the stack if @a _move is diff --git a/test/SolidityEndToEndTest.cpp b/test/SolidityEndToEndTest.cpp index 6c7b75027..65a77a543 100644 --- a/test/SolidityEndToEndTest.cpp +++ b/test/SolidityEndToEndTest.cpp @@ -882,6 +882,18 @@ BOOST_AUTO_TEST_CASE(constructor) testSolidityAgainstCpp("get(uint256)", get, u256(7)); } +BOOST_AUTO_TEST_CASE(simple_accessor) +{ + char const* sourceCode = "contract test {\n" + " uint256 data;\n" + " function test() {\n" + " data = 8;\n" + " }\n" + "}\n"; + compileAndRun(sourceCode); + BOOST_CHECK(callContractFunction("data()") == encodeArgs(8)); +} + BOOST_AUTO_TEST_CASE(balance) { char const* sourceCode = "contract test {\n" From 9d809f552095de56d08b17d5a64ca649c167c769 Mon Sep 17 00:00:00 2001 From: Lefteris Karapetsas Date: Wed, 28 Jan 2015 10:13:51 +0100 Subject: [PATCH 14/18] Multiple elementary state variable accessors test --- test/SolidityEndToEndTest.cpp | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/test/SolidityEndToEndTest.cpp b/test/SolidityEndToEndTest.cpp index 65a77a543..1ddb22731 100644 --- a/test/SolidityEndToEndTest.cpp +++ b/test/SolidityEndToEndTest.cpp @@ -894,6 +894,31 @@ BOOST_AUTO_TEST_CASE(simple_accessor) BOOST_CHECK(callContractFunction("data()") == encodeArgs(8)); } +BOOST_AUTO_TEST_CASE(multiple_elementary_accessors) +{ + char const* sourceCode = "contract test {\n" + " uint256 data;\n" + " string6 name;\n" + " hash a_hash;\n" + " address an_address;\n" + " function test() {\n" + " data = 8;\n" + " name = \"Celina\";\n" + " a_hash = sha3(123);\n" + " an_address = address(0x1337);\n" + " super_secret_data = 42;\n" + " }\n" + " private:" + " uint256 super_secret_data;" + "}\n"; + compileAndRun(sourceCode); + BOOST_CHECK(callContractFunction("data()") == encodeArgs(8)); + BOOST_CHECK(callContractFunction("name()") == encodeArgs("Celina")); + BOOST_CHECK(callContractFunction("a_hash()") == encodeArgs(dev::sha3(toBigEndian(u256(123))))); + BOOST_CHECK(callContractFunction("an_address()") == encodeArgs(toBigEndian(u160(0x1337)))); + BOOST_CHECK(!(callContractFunction("super_secret_data()") == encodeArgs(42))); +} + BOOST_AUTO_TEST_CASE(balance) { char const* sourceCode = "contract test {\n" From eb51218af41a043c34f01900719f59b5a45df9f7 Mon Sep 17 00:00:00 2001 From: Lefteris Karapetsas Date: Wed, 28 Jan 2015 13:16:09 +0100 Subject: [PATCH 15/18] Simplify FunctionType's Vardecl constructor --- libsolidity/Types.cpp | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/libsolidity/Types.cpp b/libsolidity/Types.cpp index 812271e30..fcb10d4b5 100644 --- a/libsolidity/Types.cpp +++ b/libsolidity/Types.cpp @@ -608,17 +608,12 @@ FunctionType::FunctionType(FunctionDefinition const& _function, bool _isInternal FunctionType::FunctionType(VariableDeclaration const& _varDecl): m_location(Location::EXTERNAL) { - TypePointers params; - vector paramNames; - TypePointers retParams; - vector retParamNames; + TypePointers params({}); + vector paramNames({}); + TypePointers retParams({_varDecl.getType()}); + vector retParamNames({ _varDecl.getName()}); // for now, no input parameters LTODO: change for some things like mapping - params.reserve(0); - paramNames.reserve(0); - retParams.reserve(1); - retParamNames.reserve(1); - retParams.push_back(_varDecl.getType()); - retParamNames.push_back(_varDecl.getName()); + swap(params, m_parameterTypes); swap(paramNames, m_parameterNames); swap(retParams, m_returnParameterTypes); From ffa06e1a0549d58677aa31da6ef523dabe6c27cd Mon Sep 17 00:00:00 2001 From: Lefteris Karapetsas Date: Wed, 28 Jan 2015 14:19:47 +0100 Subject: [PATCH 16/18] Explicitly specify insertion to exported functions --- libsolidity/AST.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libsolidity/AST.cpp b/libsolidity/AST.cpp index 71bf7c4ab..a23846a05 100644 --- a/libsolidity/AST.cpp +++ b/libsolidity/AST.cpp @@ -83,7 +83,7 @@ map, FunctionDescription> ContractDefinition::getInterfaceFunctions map, FunctionDescription> exportedFunctions; for (auto const& it: exportedFunctionList) - exportedFunctions[std::get<0>(it)] = std::move(FunctionDescription(std::get<1>(it), std::get<2>(it))); + exportedFunctions.insert(make_pair(std::get<0>(it), FunctionDescription(std::get<1>(it), std::get<2>(it)))); solAssert(exportedFunctionList.size() == exportedFunctions.size(), "Hash collision at Function Definition Hash calculation"); From 762aa2d0f39096ae2c6b3193d7e5938f297015c3 Mon Sep 17 00:00:00 2001 From: Lefteris Karapetsas Date: Wed, 28 Jan 2015 16:48:27 +0100 Subject: [PATCH 17/18] Function name clashing with Statevariable accessor test --- test/SolidityNameAndTypeResolution.cpp | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/SolidityNameAndTypeResolution.cpp b/test/SolidityNameAndTypeResolution.cpp index 3b711bfec..bc0edb504 100644 --- a/test/SolidityNameAndTypeResolution.cpp +++ b/test/SolidityNameAndTypeResolution.cpp @@ -650,6 +650,18 @@ BOOST_AUTO_TEST_CASE(state_variable_accessors) BOOST_CHECK(function.isConstant()); } +BOOST_AUTO_TEST_CASE(function_clash_with_state_variable_accessor) +{ + char const* text = "contract test {\n" + " function fun() {\n" + " uint64(2);\n" + " }\n" + "uint256 foo;\n" + " function foo() {}\n" + "}\n"; + BOOST_CHECK_THROW(parseTextAndResolveNames(text), DeclarationError); +} + BOOST_AUTO_TEST_CASE(private_state_variable) { char const* text = "contract test {\n" From 2fcfb45760d8980fa415a8d31fa161e21a5303fe Mon Sep 17 00:00:00 2001 From: Lefteris Karapetsas Date: Wed, 28 Jan 2015 18:06:45 +0100 Subject: [PATCH 18/18] Various fixes pertaining to State Variable accessors --- libsolidity/AST.cpp | 6 +++--- libsolidity/AST.h | 11 ++++++----- libsolidity/Compiler.cpp | 10 +++++----- libsolidity/CompilerContext.h | 3 --- libsolidity/ExpressionCompiler.cpp | 13 +++++++------ libsolidity/ExpressionCompiler.h | 8 ++++---- test/SolidityNameAndTypeResolution.cpp | 4 ++-- 7 files changed, 27 insertions(+), 28 deletions(-) diff --git a/libsolidity/AST.cpp b/libsolidity/AST.cpp index a23846a05..d95a254e9 100644 --- a/libsolidity/AST.cpp +++ b/libsolidity/AST.cpp @@ -155,7 +155,7 @@ vector, std::shared_ptr, Declaration cons } for (ASTPointer const& v: contract->getStateVariables()) - if (v->isPublic()) + if (v->isPublic() && functionsSeen.count(v->getName()) == 0) { FunctionType ftype(*v); functionsSeen.insert(v->getName()); @@ -519,12 +519,12 @@ void Literal::checkTypeRequirements() BOOST_THROW_EXCEPTION(createTypeError("Invalid literal value.")); } -std::string ParamDescription::getName() const +std::string const& ParamDescription::getName() const { return m_description.first; } -std::string ParamDescription::getType() const +std::string const& ParamDescription::getType() const { return m_description.second; } diff --git a/libsolidity/AST.h b/libsolidity/AST.h index a99bc39cb..f397c13ad 100755 --- a/libsolidity/AST.h +++ b/libsolidity/AST.h @@ -158,16 +158,16 @@ private: /** -* Generic Parameter description used by @see FunctionDescription to return -* a descripton of its parameters. -*/ + * Generic Parameter description used by @see FunctionDescription to return + * a descripton of its parameters. + */ struct ParamDescription { ParamDescription(std::string const& _name, std::string const& _type): m_description(_name, _type){} - std::string getName() const; - std::string getType() const; + std::string const& getName() const; + std::string const& getType() const; std::pair m_description; }; @@ -456,6 +456,7 @@ private: ASTPointer m_typeName; ///< can be empty ("var") bool m_isPublic; ///< Whether there is an accessor for it or not bool m_isStateVariable; ///< Whether or not this is a contract state variable + std::shared_ptr m_type; ///< derived type, initially empty }; diff --git a/libsolidity/Compiler.cpp b/libsolidity/Compiler.cpp index bda18bc68..c7656363a 100644 --- a/libsolidity/Compiler.cpp +++ b/libsolidity/Compiler.cpp @@ -51,7 +51,7 @@ void Compiler::compileContract(ContractDefinition const& _contract, if (vardecl->isPublic()) m_context.addFunction(*vardecl); - for (ASTPointer const& modifier: contract->getFunctionModifiers()) + for (ASTPointer const& modifier: contract->getFunctionModifiers()) m_context.addModifier(*modifier); } @@ -295,15 +295,15 @@ void Compiler::registerStateVariables(ContractDefinition const& _contract) void Compiler::generateAccessorCode(VariableDeclaration const& _varDecl) { m_context.startNewFunction(); - m_returnTag = m_context.newTag(); m_breakTags.clear(); m_continueTags.clear(); m_context << m_context.getFunctionEntryLabel(_varDecl); - ExpressionCompiler::appendStateVariableAccessor(m_context, &_varDecl); + ExpressionCompiler::appendStateVariableAccessor(m_context, _varDecl); - uint64_t foo = uint64_t(_varDecl.getType()->getStorageSize()); - m_context << eth::dupInstruction(foo + 1); + unsigned sizeOnStack = _varDecl.getType()->getSizeOnStack(); + solAssert(sizeOnStack <= 15, "Illegal variable stack size detected"); + m_context << eth::dupInstruction(sizeOnStack + 1); m_context << eth::Instruction::JUMP; } diff --git a/libsolidity/CompilerContext.h b/libsolidity/CompilerContext.h index 42ac9ee82..9de3385a6 100644 --- a/libsolidity/CompilerContext.h +++ b/libsolidity/CompilerContext.h @@ -117,10 +117,7 @@ private: std::map m_stateVariables; /// Offsets of local variables on the stack (relative to stack base). std::map m_localVariables; - /// Sum of stack sizes of local variables - unsigned m_localVariablesSize; /// Labels pointing to the entry points of functions. - std::map m_functionEntryLabels; /// Labels pointing to the entry points of function overrides. std::map m_virtualFunctionEntryLabels; diff --git a/libsolidity/ExpressionCompiler.cpp b/libsolidity/ExpressionCompiler.cpp index 66e2c68cc..15ee17fd3 100644 --- a/libsolidity/ExpressionCompiler.cpp +++ b/libsolidity/ExpressionCompiler.cpp @@ -48,7 +48,7 @@ void ExpressionCompiler::appendTypeConversion(CompilerContext& _context, Type co compiler.appendTypeConversion(_typeOnStack, _targetType, _cleanupNeeded); } -void ExpressionCompiler::appendStateVariableAccessor(CompilerContext& _context, VariableDeclaration const* _varDecl, bool _optimize) +void ExpressionCompiler::appendStateVariableAccessor(CompilerContext& _context, VariableDeclaration const& _varDecl, bool _optimize) { ExpressionCompiler compiler(_context, _optimize); compiler.appendStateVariableAccessor(_varDecl); @@ -795,10 +795,11 @@ unsigned ExpressionCompiler::appendArgumentCopyToMemory(TypePointers const& _typ return length; } -void ExpressionCompiler::appendStateVariableAccessor(VariableDeclaration const* _varDecl) +void ExpressionCompiler::appendStateVariableAccessor(VariableDeclaration const& _varDecl) { - m_currentLValue.fromStateVariable(*_varDecl, _varDecl->getType()); - m_currentLValue.retrieveValueFromStorage(_varDecl->getType(), true); + m_currentLValue.fromStateVariable(_varDecl, _varDecl.getType()); + solAssert(m_currentLValue.isInStorage(), ""); + m_currentLValue.retrieveValueFromStorage(_varDecl.getType(), true); } ExpressionCompiler::LValue::LValue(CompilerContext& _compilerContext, LValueType _type, Type const& _dataType, @@ -843,7 +844,7 @@ void ExpressionCompiler::LValue::retrieveValue(Expression const& _expression, bo } } -void ExpressionCompiler::LValue::retrieveValueFromStorage(std::shared_ptr const& _type, bool _remove) const +void ExpressionCompiler::LValue::retrieveValueFromStorage(TypePointer const& _type, bool _remove) const { if (!_type->isValueType()) return; // no distinction between value and reference for non-value types @@ -968,7 +969,7 @@ void ExpressionCompiler::LValue::retrieveValueIfLValueNotRequested(Expression co } } -void ExpressionCompiler::LValue::fromStateVariable(Declaration const& _varDecl, std::shared_ptr const& _type) +void ExpressionCompiler::LValue::fromStateVariable(Declaration const& _varDecl, TypePointer const& _type) { m_type = STORAGE; solAssert(_type->getStorageSize() <= numeric_limits::max(), "The storage size of " + _type->toString() + " should fit in an unsigned"); diff --git a/libsolidity/ExpressionCompiler.h b/libsolidity/ExpressionCompiler.h index 8479344ad..b4a64594d 100644 --- a/libsolidity/ExpressionCompiler.h +++ b/libsolidity/ExpressionCompiler.h @@ -54,7 +54,7 @@ public: static void appendTypeConversion(CompilerContext& _context, Type const& _typeOnStack, Type const& _targetType, bool _cleanupNeeded = false); /// Appends code for a State Variable accessor function - static void appendStateVariableAccessor(CompilerContext& _context, VariableDeclaration const* _varDecl, bool _optimize = false); + static void appendStateVariableAccessor(CompilerContext& _context, VariableDeclaration const& _varDecl, bool _optimize = false); private: explicit ExpressionCompiler(CompilerContext& _compilerContext, bool _optimize = false): @@ -98,7 +98,7 @@ private: unsigned _memoryOffset = 0); /// Appends code for a State Variable accessor function - void appendStateVariableAccessor(VariableDeclaration const* _varDecl); + void appendStateVariableAccessor(VariableDeclaration const& _varDecl); /** * Helper class to store and retrieve lvalues to and from various locations. @@ -117,7 +117,7 @@ private: /// @a _expression is the current expression void fromIdentifier(Identifier const& _identifier, Declaration const& _declaration); /// Convenience function to set type for a state variable and retrieve the reference - void fromStateVariable(Declaration const& _varDecl, std::shared_ptr const& _type); + void fromStateVariable(Declaration const& _varDecl, TypePointer const& _type); void reset() { m_type = NONE; m_baseStackOffset = 0; m_size = 0; } bool isValid() const { return m_type != NONE; } @@ -133,7 +133,7 @@ private: /// @a _expression is the current expression, used for error reporting. void retrieveValue(Expression const& _expression, bool _remove = false) const; /// Convenience function to retrieve Value from Storage. Specific version of @ref retrieveValue - void retrieveValueFromStorage(std::shared_ptr const& _type, bool _remove = false) const; + void retrieveValueFromStorage(TypePointer const& _type, bool _remove = false) const; /// Stores a value (from the stack directly beneath the reference, which is assumed to /// be on the top of the stack, if any) in the lvalue and removes the reference. /// Also removes the stored value from the stack if @a _move is diff --git a/test/SolidityNameAndTypeResolution.cpp b/test/SolidityNameAndTypeResolution.cpp index bc0edb504..979836ecc 100644 --- a/test/SolidityNameAndTypeResolution.cpp +++ b/test/SolidityNameAndTypeResolution.cpp @@ -76,7 +76,7 @@ ASTPointer parseTextAndResolveNamesWithChecks(std::string const& _so } catch(boost::exception const& _e) { - auto msg = std::string("Parsing text and resolving nanes failed with: \n") + boost::diagnostic_information(_e); + auto msg = std::string("Parsing text and resolving names failed with: \n") + boost::diagnostic_information(_e); BOOST_FAIL(msg); } return sourceUnit; @@ -642,7 +642,7 @@ BOOST_AUTO_TEST_CASE(state_variable_accessors) ASTPointer source; ContractDefinition const* contract; BOOST_CHECK_NO_THROW(source = parseTextAndResolveNamesWithChecks(text)); - BOOST_CHECK((contract = retrieveContract(source, 0)) != nullptr); + BOOST_REQUIRE((contract = retrieveContract(source, 0)) != nullptr); FunctionDescription function = retrieveFunctionBySignature(contract, "foo()"); BOOST_CHECK_MESSAGE(function.getDeclaration() != nullptr, "Could not find the accessor function"); auto returnParams = function.getReturnParameters();