From df4746d5e7cbef44726eea23cbf68f2a10232a7f Mon Sep 17 00:00:00 2001 From: Liana Husikyan Date: Fri, 27 Feb 2015 17:41:22 +0100 Subject: [PATCH 1/2] Implemented passing arguments to the base constructor. --- libsolidity/AST.cpp | 31 ++++++--- libsolidity/AST.h | 5 +- libsolidity/Compiler.cpp | 108 ++++++++++++++++++++------------ libsolidity/Compiler.h | 7 ++- libsolidity/CompilerContext.cpp | 33 +++++++--- libsolidity/CompilerContext.h | 8 ++- test/SolidityEndToEndTest.cpp | 64 +++++++++++++++++++ 7 files changed, 196 insertions(+), 60 deletions(-) diff --git a/libsolidity/AST.cpp b/libsolidity/AST.cpp index 4c6db6a20..00660c5da 100644 --- a/libsolidity/AST.cpp +++ b/libsolidity/AST.cpp @@ -308,7 +308,9 @@ void FunctionDefinition::checkTypeRequirements() if (!var->getType()->canLiveOutsideStorage()) BOOST_THROW_EXCEPTION(var->createTypeError("Type is required to live outside storage.")); for (ASTPointer const& modifier: m_functionModifiers) - modifier->checkTypeRequirements(); + modifier->checkTypeRequirements(isConstructor() ? + dynamic_cast(*getScope()).getBaseContracts() : + vector>()); m_body->checkTypeRequirements(); } @@ -351,19 +353,34 @@ void ModifierDefinition::checkTypeRequirements() m_body->checkTypeRequirements(); } -void ModifierInvocation::checkTypeRequirements() +void ModifierInvocation::checkTypeRequirements(std::vector> const& _bases) { m_modifierName->checkTypeRequirements(); for (ASTPointer const& argument: m_arguments) argument->checkTypeRequirements(); - ModifierDefinition const* modifier = dynamic_cast(m_modifierName->getReferencedDeclaration()); - solAssert(modifier, "Function modifier not found."); - vector> const& parameters = modifier->getParameters(); - if (parameters.size() != m_arguments.size()) + auto declaration = m_modifierName->getReferencedDeclaration(); + vector> emptyParameterList; + vector> const* parameters = nullptr; + if (auto modifier = dynamic_cast(declaration)) + parameters = &modifier->getParameters(); + else + for (auto const& base: _bases) + if (declaration == base->getName()->getReferencedDeclaration()) + { + m_referencedConstructor = dynamic_cast(*declaration).getConstructor(); + if (m_referencedConstructor) + parameters = &m_referencedConstructor->getParameters(); + else + parameters = &emptyParameterList; + break; + } + if (!parameters) + BOOST_THROW_EXCEPTION(createTypeError("Referenced declaration is neither modifier nor base class.")); + if (parameters->size() != m_arguments.size()) BOOST_THROW_EXCEPTION(createTypeError("Wrong argument count for modifier invocation.")); for (size_t i = 0; i < m_arguments.size(); ++i) - if (!m_arguments[i]->getType()->isImplicitlyConvertibleTo(*parameters[i]->getType())) + if (!m_arguments[i]->getType()->isImplicitlyConvertibleTo(*(*parameters)[i]->getType())) BOOST_THROW_EXCEPTION(createTypeError("Invalid type for argument in modifier invocation.")); } diff --git a/libsolidity/AST.h b/libsolidity/AST.h index d028e8a76..967c6792d 100644 --- a/libsolidity/AST.h +++ b/libsolidity/AST.h @@ -510,7 +510,7 @@ private: }; /** - * Invocation/usage of a modifier in a function header. + * Invocation/usage of a modifier in a function header or a base constructor call. */ class ModifierInvocation: public ASTNode { @@ -525,11 +525,12 @@ public: ASTPointer const& getName() const { return m_modifierName; } std::vector> const& getArguments() const { return m_arguments; } - void checkTypeRequirements(); + void checkTypeRequirements(std::vector> const& _bases); private: ASTPointer m_modifierName; std::vector> m_arguments; + FunctionDefinition const* m_referencedConstructor = nullptr; }; /** diff --git a/libsolidity/Compiler.cpp b/libsolidity/Compiler.cpp index 2f75d2ea4..580fd5b62 100644 --- a/libsolidity/Compiler.cpp +++ b/libsolidity/Compiler.cpp @@ -58,7 +58,10 @@ void Compiler::compileContract(ContractDefinition const& _contract, while (!functions.empty()) { for (Declaration const* function: functions) + { + m_context.setStackOffset(0); function->accept(*this); + } functions = m_context.getFunctionsWithoutCode(); } @@ -79,37 +82,38 @@ void Compiler::initializeContext(ContractDefinition const& _contract, void Compiler::packIntoContractCreator(ContractDefinition const& _contract, CompilerContext const& _runtimeContext) { - // arguments for base constructors, filled in derived-to-base order - map> const*> baseArguments; - // Determine the arguments that are used for the base constructors. std::vector const& bases = _contract.getLinearizedBaseContracts(); for (ContractDefinition const* contract: bases) + { + if (FunctionDefinition const* constructor = contract->getConstructor()) + for (auto const& modifier: constructor->getModifiers()) + { + auto baseContract = dynamic_cast( + modifier->getName()->getReferencedDeclaration()); + if (baseContract) + if (m_baseArguments.count(baseContract->getConstructor()) == 0) + m_baseArguments[baseContract->getConstructor()] = &modifier->getArguments(); + } + for (ASTPointer const& base: contract->getBaseContracts()) { ContractDefinition const* baseContract = dynamic_cast( base->getName()->getReferencedDeclaration()); solAssert(baseContract, ""); - if (baseArguments.count(baseContract) == 0) - baseArguments[baseContract] = &base->getArguments(); - } - // Call constructors in base-to-derived order. - // The Constructor for the most derived contract is called later. - for (unsigned i = 1; i < bases.size(); i++) - { - ContractDefinition const* base = bases[bases.size() - i]; - solAssert(base, ""); - initializeStateVariables(*base); - FunctionDefinition const* baseConstructor = base->getConstructor(); - if (!baseConstructor) - continue; - solAssert(baseArguments[base], ""); - appendBaseConstructorCall(*baseConstructor, *baseArguments[base]); + if (m_baseArguments.count(baseContract->getConstructor()) == 0) + m_baseArguments[baseContract->getConstructor()] = &base->getArguments(); + } } - initializeStateVariables(_contract); - if (_contract.getConstructor()) - appendConstructorCall(*_contract.getConstructor()); + // Initialization of state variables in base-to-derived order. + for (ContractDefinition const* contract: boost::adaptors::reverse(bases)) + initializeStateVariables(*contract); + + if (FunctionDefinition const* constructor = _contract.getConstructor()) + appendConstructor(*constructor); + else if (auto c = m_context.getNextConstructor(_contract)) + appendBaseConstructor(*c); eth::AssemblyItem sub = m_context.addSubroutine(_runtimeContext.getAssembly()); // stack contains sub size @@ -126,22 +130,23 @@ void Compiler::packIntoContractCreator(ContractDefinition const& _contract, Comp } } -void Compiler::appendBaseConstructorCall(FunctionDefinition const& _constructor, - vector> const& _arguments) +void Compiler::appendBaseConstructor(FunctionDefinition const& _constructor) { CompilerContext::LocationSetter locationSetter(m_context, &_constructor); FunctionType constructorType(_constructor); - eth::AssemblyItem returnLabel = m_context.pushNewTag(); - for (unsigned i = 0; i < _arguments.size(); ++i) - compileExpression(*_arguments[i], constructorType.getParameterTypes()[i]); - m_context.appendJumpTo(m_context.getFunctionEntryLabel(_constructor)); - m_context << returnLabel; + if (!constructorType.getParameterTypes().empty()) + { + std::vector> const* arguments = m_baseArguments[&_constructor]; + solAssert(arguments, ""); + for (unsigned i = 0; i < arguments->size(); ++i) + compileExpression(*(arguments->at(i)), constructorType.getParameterTypes()[i]); + } + _constructor.accept(*this); } -void Compiler::appendConstructorCall(FunctionDefinition const& _constructor) +void Compiler::appendConstructor(FunctionDefinition const& _constructor) { CompilerContext::LocationSetter locationSetter(m_context, &_constructor); - eth::AssemblyItem returnTag = m_context.pushNewTag(); // copy constructor arguments from code to memory and then to stack, they are supplied after the actual program unsigned argumentSize = 0; for (ASTPointer const& var: _constructor.getParameters()) @@ -155,8 +160,7 @@ void Compiler::appendConstructorCall(FunctionDefinition const& _constructor) m_context << eth::Instruction::CODECOPY; appendCalldataUnpacker(FunctionType(_constructor).getParameterTypes(), true); } - m_context.appendJumpTo(m_context.getFunctionEntryLabel(_constructor)); - m_context << returnTag; + _constructor.accept(*this); } void Compiler::appendFunctionSelector(ContractDefinition const& _contract) @@ -296,28 +300,36 @@ bool Compiler::visit(FunctionDefinition const& _function) // although note that this reduces the size of the visible stack m_context.startFunction(_function); - m_returnTag = m_context.newTag(); - m_breakTags.clear(); - m_continueTags.clear(); - m_stackCleanupForReturn = 0; - m_currentFunction = &_function; - m_modifierDepth = 0; // stack upon entry: [return address] [arg0] [arg1] ... [argn] // reserve additional slots: [retarg0] ... [retargm] [localvar0] ... [localvarp] unsigned parametersSize = CompilerUtils::getSizeOnStack(_function.getParameters()); - m_context.adjustStackOffset(parametersSize); + if (!_function.isConstructor()) + // adding 1 for return address. + m_context.adjustStackOffset(parametersSize + 1); for (ASTPointer const& variable: _function.getParameters()) { m_context.addVariable(*variable, parametersSize); parametersSize -= variable->getType()->getSizeOnStack(); } + for (ASTPointer const& variable: _function.getReturnParameters()) m_context.addAndInitializeVariable(*variable); for (VariableDeclaration const* localVariable: _function.getLocalVariables()) m_context.addAndInitializeVariable(*localVariable); + if (_function.isConstructor()) + if (auto c = m_context.getNextConstructor(dynamic_cast(*_function.getScope()))) + appendBaseConstructor(*c); + + m_returnTag = m_context.newTag(); + m_breakTags.clear(); + m_continueTags.clear(); + m_stackCleanupForReturn = 0; + m_currentFunction = &_function; + m_modifierDepth = 0; + appendModifierOrFunctionCode(); m_context << m_returnTag; @@ -352,8 +364,14 @@ bool Compiler::visit(FunctionDefinition const& _function) } //@todo assert that everything is in place now - m_context << eth::Instruction::JUMP; + for (ASTPointer const& variable: _function.getParameters() + _function.getReturnParameters()) + m_context.removeVariable(*variable); + for (VariableDeclaration const* localVariable: _function.getLocalVariables()) + m_context.removeVariable(*localVariable); + m_context.adjustStackOffset(-c_returnValuesSize); + if (!_function.isConstructor()) + m_context << eth::Instruction::JUMP; return false; } @@ -515,6 +533,16 @@ void Compiler::appendModifierOrFunctionCode() else { ASTPointer const& modifierInvocation = m_currentFunction->getModifiers()[m_modifierDepth]; + + // constructor call should be excluded + if (dynamic_cast(modifierInvocation->getName()->getReferencedDeclaration())) + { + ++m_modifierDepth; + appendModifierOrFunctionCode(); + --m_modifierDepth; + return; + } + ModifierDefinition const& modifier = m_context.getFunctionModifier(modifierInvocation->getName()->getName()); CompilerContext::LocationSetter locationSetter(m_context, &modifier); solAssert(modifier.getParameters().size() == modifierInvocation->getArguments().size(), ""); diff --git a/libsolidity/Compiler.h b/libsolidity/Compiler.h index 3ad2d8c61..2804e8eca 100644 --- a/libsolidity/Compiler.h +++ b/libsolidity/Compiler.h @@ -55,9 +55,8 @@ private: /// Adds the code that is run at creation time. Should be run after exchanging the run-time context /// with a new and initialized context. Adds the constructor code. void packIntoContractCreator(ContractDefinition const& _contract, CompilerContext const& _runtimeContext); - void appendBaseConstructorCall(FunctionDefinition const& _constructor, - std::vector> const& _arguments); - void appendConstructorCall(FunctionDefinition const& _constructor); + void appendBaseConstructor(FunctionDefinition const& _constructor); + void appendConstructor(FunctionDefinition const& _constructor); void appendFunctionSelector(ContractDefinition const& _contract); /// 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. @@ -94,6 +93,8 @@ private: unsigned m_modifierDepth = 0; FunctionDefinition const* m_currentFunction; unsigned m_stackCleanupForReturn; ///< this number of stack elements need to be removed before jump to m_returnTag + // arguments for base constructors, filled in derived-to-base order + std::map> const*> m_baseArguments; }; } diff --git a/libsolidity/CompilerContext.cpp b/libsolidity/CompilerContext.cpp index 18be337fa..7184dc37d 100644 --- a/libsolidity/CompilerContext.cpp +++ b/libsolidity/CompilerContext.cpp @@ -51,8 +51,6 @@ void CompilerContext::addStateVariable(VariableDeclaration const& _declaration) void CompilerContext::startFunction(Declaration const& _function) { m_functionsWithCode.insert(&_function); - m_localVariables.clear(); - m_asm.setDeposit(0); *this << getFunctionEntryLabel(_function); } @@ -63,6 +61,12 @@ void CompilerContext::addVariable(VariableDeclaration const& _declaration, m_localVariables[&_declaration] = unsigned(m_asm.deposit()) - _offsetToCurrent; } +void CompilerContext::removeVariable(VariableDeclaration const& _declaration) +{ + solAssert(m_localVariables.count(&_declaration), ""); + m_localVariables.erase(&_declaration); +} + void CompilerContext::addAndInitializeVariable(VariableDeclaration const& _declaration) { LocationSetter locationSetter(*this, &_declaration); @@ -110,11 +114,8 @@ eth::AssemblyItem CompilerContext::getVirtualFunctionEntryLabel(FunctionDefiniti eth::AssemblyItem CompilerContext::getSuperFunctionEntryLabel(string const& _name, ContractDefinition const& _base) { - // search for first contract after _base - solAssert(!m_inheritanceHierarchy.empty(), "No inheritance hierarchy set."); - auto it = find(m_inheritanceHierarchy.begin(), m_inheritanceHierarchy.end(), &_base); - solAssert(it != m_inheritanceHierarchy.end(), "Base not found in inheritance hierarchy."); - for (++it; it != m_inheritanceHierarchy.end(); ++it) + auto it = getSuperContract(_base); + for (; it != m_inheritanceHierarchy.end(); ++it) for (ASTPointer const& function: (*it)->getDefinedFunctions()) if (!function->isConstructor() && function->getName() == _name) return getFunctionEntryLabel(*function); @@ -122,6 +123,16 @@ eth::AssemblyItem CompilerContext::getSuperFunctionEntryLabel(string const& _nam return m_asm.newTag(); // not reached } +FunctionDefinition const* CompilerContext::getNextConstructor(ContractDefinition const& _contract) const +{ + vector::const_iterator it = getSuperContract(_contract); + for (; it != m_inheritanceHierarchy.end(); it = getSuperContract(**it)) + if ((*it)->getConstructor()) + return (*it)->getConstructor(); + + return nullptr; +} + set CompilerContext::getFunctionsWithoutCode() { set functions; @@ -201,5 +212,13 @@ CompilerContext& CompilerContext::operator<<(bytes const& _data) return *this; } +vector::const_iterator CompilerContext::getSuperContract(ContractDefinition const& _contract) const +{ + solAssert(!m_inheritanceHierarchy.empty(), "No inheritance hierarchy set."); + auto it = find(m_inheritanceHierarchy.begin(), m_inheritanceHierarchy.end(), &_contract); + solAssert(it != m_inheritanceHierarchy.end(), "Base not found in inheritance hierarchy."); + return ++it; +} + } } diff --git a/libsolidity/CompilerContext.h b/libsolidity/CompilerContext.h index 94d6443e9..301ef1468 100644 --- a/libsolidity/CompilerContext.h +++ b/libsolidity/CompilerContext.h @@ -43,11 +43,13 @@ public: void addMagicGlobal(MagicVariableDeclaration const& _declaration); void addStateVariable(VariableDeclaration const& _declaration); void addVariable(VariableDeclaration const& _declaration, unsigned _offsetToCurrent = 0); + void removeVariable(VariableDeclaration const& _declaration); void addAndInitializeVariable(VariableDeclaration const& _declaration); void setCompiledContracts(std::map const& _contracts) { m_compiledContracts = _contracts; } bytes const& getCompiledContract(ContractDefinition const& _contract) const; + void setStackOffset(int _offset) { m_asm.setDeposit(_offset); } void adjustStackOffset(int _adjustment) { m_asm.adjustDeposit(_adjustment); } unsigned getStackHeight() const { solAssert(m_asm.deposit() >= 0, ""); return unsigned(m_asm.deposit()); } @@ -62,6 +64,8 @@ public: /// @returns the entry label of function with the given name from the most derived class just /// above _base in the current inheritance hierarchy. eth::AssemblyItem getSuperFunctionEntryLabel(std::string const& _name, ContractDefinition const& _base); + FunctionDefinition const* getNextConstructor(ContractDefinition const& _contract) const; + /// @returns the set of functions for which we still need to generate code std::set getFunctionsWithoutCode(); /// Resets function specific members, inserts the function entry label and marks the function @@ -126,9 +130,11 @@ public: LocationSetter(CompilerContext& _compilerContext, ASTNode const* _node): ScopeGuard(std::bind(&CompilerContext::popVisitedNodes, _compilerContext)) { _compilerContext.pushVisitedNodes(_node); } }; - eth::Assembly m_asm; + private: + std::vector::const_iterator getSuperContract(const ContractDefinition &_contract) const; + eth::Assembly m_asm; /// Magic global variables like msg, tx or this, distinguished by type. std::set m_magicGlobals; /// Other already compiled contracts to be used in contract creation calls. diff --git a/test/SolidityEndToEndTest.cpp b/test/SolidityEndToEndTest.cpp index f305e81e3..87a8eb79f 100644 --- a/test/SolidityEndToEndTest.cpp +++ b/test/SolidityEndToEndTest.cpp @@ -2946,6 +2946,70 @@ BOOST_AUTO_TEST_CASE(array_copy_storage_storage_struct) BOOST_CHECK(m_state.storage(m_contractAddress).empty()); } +BOOST_AUTO_TEST_CASE(pass_dynamic_arguments_to_the_base) +{ + char const* sourceCode = R"( + contract Base { + function Base(uint i) + { + m_i = i; + } + uint public m_i; + } + contract Derived is Base(2) { + function Derived(uint i) Base(i) + {} + } + contract Final is Derived(4) { + })"; + compileAndRun(sourceCode); + BOOST_CHECK(callContractFunction("m_i()") == encodeArgs(4)); +} + +BOOST_AUTO_TEST_CASE(pass_dynamic_arguments_to_the_base_base) +{ + char const* sourceCode = R"( + contract Base { + function Base(uint j) + { + m_i = j; + } + uint public m_i; + } + contract Base1 is Base(3) { + function Base1(uint k) Base(k*k) {} + } + contract Derived is Base(3), Base1(2) { + function Derived(uint i) Base(i) Base1(i) + {} + } + contract Final is Derived(4) { + })"; + compileAndRun(sourceCode); + BOOST_CHECK(callContractFunction("m_i()") == encodeArgs(4)); +} + +BOOST_AUTO_TEST_CASE(pass_dynamic_arguments_to_the_base_base_with_gap) +{ + char const* sourceCode = R"( + contract Base { + function Base(uint i) + { + m_i = i; + } + uint public m_i; + } + contract Base1 is Base(3) {} + contract Derived is Base(2), Base1 { + function Derived(uint i) Base(i) {} + } + contract Final is Derived(4) { + })"; + compileAndRun(sourceCode); + BOOST_CHECK(callContractFunction("m_i()") == encodeArgs(4)); +} + + BOOST_AUTO_TEST_SUITE_END() } From a545b8a7a46c20940f436226ea67c6f23cffa4cd Mon Sep 17 00:00:00 2001 From: Liana Husikyan Date: Mon, 2 Mar 2015 14:21:12 +0100 Subject: [PATCH 2/2] removed unused member added some comments for ModifierInvocation::checkTypeRequirements cleanup --- libsolidity/AST.cpp | 8 ++++---- libsolidity/AST.h | 2 +- libsolidity/CompilerContext.cpp | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/libsolidity/AST.cpp b/libsolidity/AST.cpp index 00660c5da..aba355768 100644 --- a/libsolidity/AST.cpp +++ b/libsolidity/AST.cpp @@ -353,7 +353,7 @@ void ModifierDefinition::checkTypeRequirements() m_body->checkTypeRequirements(); } -void ModifierInvocation::checkTypeRequirements(std::vector> const& _bases) +void ModifierInvocation::checkTypeRequirements(vector> const& _bases) { m_modifierName->checkTypeRequirements(); for (ASTPointer const& argument: m_arguments) @@ -365,12 +365,12 @@ void ModifierInvocation::checkTypeRequirements(std::vector(declaration)) parameters = &modifier->getParameters(); else + // check parameters for Base constructors for (auto const& base: _bases) if (declaration == base->getName()->getReferencedDeclaration()) { - m_referencedConstructor = dynamic_cast(*declaration).getConstructor(); - if (m_referencedConstructor) - parameters = &m_referencedConstructor->getParameters(); + if (auto referencedConstructor = dynamic_cast(*declaration).getConstructor()) + parameters = &referencedConstructor->getParameters(); else parameters = &emptyParameterList; break; diff --git a/libsolidity/AST.h b/libsolidity/AST.h index 967c6792d..c91c433ed 100644 --- a/libsolidity/AST.h +++ b/libsolidity/AST.h @@ -525,12 +525,12 @@ public: ASTPointer const& getName() const { return m_modifierName; } std::vector> const& getArguments() const { return m_arguments; } + /// @param _bases is the list of base contracts for base constructor calls. For modifiers an empty vector should be passed. void checkTypeRequirements(std::vector> const& _bases); private: ASTPointer m_modifierName; std::vector> m_arguments; - FunctionDefinition const* m_referencedConstructor = nullptr; }; /** diff --git a/libsolidity/CompilerContext.cpp b/libsolidity/CompilerContext.cpp index 7184dc37d..61c25052c 100644 --- a/libsolidity/CompilerContext.cpp +++ b/libsolidity/CompilerContext.cpp @@ -126,7 +126,7 @@ eth::AssemblyItem CompilerContext::getSuperFunctionEntryLabel(string const& _nam FunctionDefinition const* CompilerContext::getNextConstructor(ContractDefinition const& _contract) const { vector::const_iterator it = getSuperContract(_contract); - for (; it != m_inheritanceHierarchy.end(); it = getSuperContract(**it)) + for (; it != m_inheritanceHierarchy.end(); ++it) if ((*it)->getConstructor()) return (*it)->getConstructor();