- added functionality to set values to 0 when deleting structure(not for mapping)

- added unit test

Made some changes after Christian`s review on pull request
- remove/edit comments
- BoolType and ContractType return VoidType after delete

- fixed constructor_arguments test

- fixed set to 0 when deleting variable from stack
- changed test case to test that

libsolidity/ExpressionCompiler.cpp

@@ -94,13 +94,8 @@ bool ExpressionCompiler::visit(UnaryOperation const& _unaryOperation)
 		m_context << eth::Instruction::NOT;
 		break;
 	case Token::DELETE: // delete
-		// @todo semantics change for complex types
 		solAssert(m_currentLValue.isValid(), "LValue not retrieved.");
-
-		m_context << u256(0);
-		if (m_currentLValue.storesReferenceOnStack())
-			m_context << eth::Instruction::SWAP1;
-		m_currentLValue.storeValue(_unaryOperation);
+		m_currentLValue.setToZero(_unaryOperation);
 		m_currentLValue.reset();
 		break;
 	case Token::INC: // ++ (pre- or postfix)
@@ -696,9 +691,14 @@ void ExpressionCompiler::appendExternalFunctionCall(FunctionType const& _functio
 
 ExpressionCompiler::LValue::LValue(CompilerContext& _compilerContext, LValueType _type, Type const& _dataType,
 								   unsigned _baseStackOffset):
-	m_context(&_compilerContext), m_type(_type), m_baseStackOffset(_baseStackOffset),
-	m_stackSize(_dataType.getSizeOnStack())
+	m_context(&_compilerContext), m_type(_type), m_baseStackOffset(_baseStackOffset)
 {
+	//@todo change the type cast for arrays
+	solAssert(_dataType.getStorageSize() <= numeric_limits<unsigned>::max(), "The storage size of " +_dataType.toString() + " should fit in unsigned");
+	if (m_type == STORAGE)
+		m_size = unsigned(_dataType.getStorageSize());
+	else
+		m_size = unsigned(_dataType.getSizeOnStack());
 }
 
 void ExpressionCompiler::LValue::retrieveValue(Expression const& _expression, bool _remove) const
@@ -711,7 +711,7 @@ void ExpressionCompiler::LValue::retrieveValue(Expression const& _expression, bo
 		if (stackPos >= 15) //@todo correct this by fetching earlier or moving to memory
 			BOOST_THROW_EXCEPTION(CompilerError() << errinfo_sourceLocation(_expression.getLocation())
 												  << errinfo_comment("Stack too deep."));
-		for (unsigned i = 0; i < m_stackSize; ++i)
+		for (unsigned i = 0; i < m_size; ++i)
 			*m_context << eth::dupInstruction(stackPos + 1);
 		break;
 	}
@@ -720,14 +720,14 @@ void ExpressionCompiler::LValue::retrieveValue(Expression const& _expression, bo
 			break; // no distinction between value and reference for non-value types
 		if (!_remove)
 			*m_context << eth::Instruction::DUP1;
-		if (m_stackSize == 1)
+		if (m_size == 1)
 			*m_context << eth::Instruction::SLOAD;
 		else
-			for (unsigned i = 0; i < m_stackSize; ++i)
+			for (unsigned i = 0; i < m_size; ++i)
 			{
 				*m_context << eth::Instruction::DUP1 << eth::Instruction::SLOAD << eth::Instruction::SWAP1;
-				if (i + 1 < m_stackSize)
-					 *m_context << u256(1) << eth::Instruction::ADD;
+				if (i + 1 < m_size)
+					*m_context << u256(1) << eth::Instruction::ADD;
 				else
 					*m_context << eth::Instruction::POP;
 			}
@@ -751,12 +751,12 @@ void ExpressionCompiler::LValue::storeValue(Expression const& _expression, bool
 	{
 	case STACK:
 	{
-		unsigned stackDiff = m_context->baseToCurrentStackOffset(unsigned(m_baseStackOffset)) - m_stackSize + 1;
+		unsigned stackDiff = m_context->baseToCurrentStackOffset(unsigned(m_baseStackOffset)) - m_size + 1;
 		if (stackDiff > 16)
 			BOOST_THROW_EXCEPTION(CompilerError() << errinfo_sourceLocation(_expression.getLocation())
 												  << errinfo_comment("Stack too deep."));
 		else if (stackDiff > 0)
-			for (unsigned i = 0; i < m_stackSize; ++i)
+			for (unsigned i = 0; i < m_size; ++i)
 				*m_context << eth::swapInstruction(stackDiff) << eth::Instruction::POP;
 		if (!_move)
 			retrieveValue(_expression);
@@ -768,17 +768,17 @@ void ExpressionCompiler::LValue::storeValue(Expression const& _expression, bool
 		// stack layout: value value ... value ref
 		if (!_move) // copy values
 		{
-			if (m_stackSize + 1 > 16)
+			if (m_size + 1 > 16)
 				BOOST_THROW_EXCEPTION(CompilerError() << errinfo_sourceLocation(_expression.getLocation())
 													  << errinfo_comment("Stack too deep."));
-			for (unsigned i = 0; i < m_stackSize; ++i)
-				*m_context << eth::dupInstruction(m_stackSize + 1) << eth::Instruction::SWAP1;
+			for (unsigned i = 0; i < m_size; ++i)
+				*m_context << eth::dupInstruction(m_size + 1) << eth::Instruction::SWAP1;
 		}
-		if (m_stackSize > 0) // store high index value first
-			*m_context << u256(m_stackSize - 1) << eth::Instruction::ADD;
-		for (unsigned i = 0; i < m_stackSize; ++i)
+		if (m_size > 0) // store high index value first
+			*m_context << u256(m_size - 1) << eth::Instruction::ADD;
+		for (unsigned i = 0; i < m_size; ++i)
 		{
-			if (i + 1 >= m_stackSize)
+			if (i + 1 >= m_size)
 				*m_context << eth::Instruction::SSTORE;
 			else
 				// v v ... v v r+x
@@ -800,6 +800,47 @@ void ExpressionCompiler::LValue::storeValue(Expression const& _expression, bool
 	}
 }
 
+void ExpressionCompiler::LValue::setToZero(Expression const& _expression) const
+{
+	switch (m_type)
+	{
+	case STACK:
+	{
+		unsigned stackDiff = m_context->baseToCurrentStackOffset(unsigned(m_baseStackOffset));
+		if (stackDiff > 16)
+			BOOST_THROW_EXCEPTION(CompilerError() << errinfo_sourceLocation(_expression.getLocation())
+												  << errinfo_comment("Stack too deep."));
+		else if (stackDiff > 0)
+			for (unsigned i = 0; i < m_size; ++i)
+				*m_context << u256(0) << eth::swapInstruction(m_size - i) << eth::Instruction::POP;
+		break;
+	}
+	case LValue::STORAGE:
+		if (m_size == 0)
+			*m_context << eth::Instruction::POP;
+		for (unsigned i = 0; i < m_size; ++i)
+		{
+			if (i + 1 >= m_size)
+				*m_context << u256(0) << eth::Instruction::SWAP1 << eth::Instruction::SSTORE;
+			else
+				*m_context << u256(0) << eth::Instruction::DUP2 << eth::Instruction::SSTORE
+							<< u256(1) << eth::Instruction::ADD;
+		}
+		break;
+	case LValue::MEMORY:
+		if (!_expression.getType()->isValueType())
+			break; // no distinction between value and reference for non-value types
+		BOOST_THROW_EXCEPTION(InternalCompilerError() << errinfo_sourceLocation(_expression.getLocation())
+													  << errinfo_comment("Location type not yet implemented."));
+		break;
+	default:
+		BOOST_THROW_EXCEPTION(InternalCompilerError() << errinfo_sourceLocation(_expression.getLocation())
+													  << errinfo_comment("Unsupported location type."));
+		break;
+	}
+
+}
+
 void ExpressionCompiler::LValue::retrieveValueIfLValueNotRequested(Expression const& _expression)
 {
 	if (!_expression.lvalueRequested())
@@ -811,15 +852,17 @@ void ExpressionCompiler::LValue::retrieveValueIfLValueNotRequested(Expression co
 
 void ExpressionCompiler::LValue::fromIdentifier(Identifier const& _identifier, Declaration const& _declaration)
 {
-	m_stackSize = _identifier.getType()->getSizeOnStack();
 	if (m_context->isLocalVariable(&_declaration))
 	{
 		m_type = STACK;
+		m_size = _identifier.getType()->getSizeOnStack();
 		m_baseStackOffset = m_context->getBaseStackOffsetOfVariable(_declaration);
 	}
 	else if (m_context->isStateVariable(&_declaration))
 	{
 		m_type = STORAGE;
+		solAssert(_identifier.getType()->getStorageSize() <= numeric_limits<unsigned>::max(), "The storage size of " + _identifier.getType()->toString() + " should fit in unsigned");
+		m_size = unsigned(_identifier.getType()->getStorageSize());
 		*m_context << m_context->getStorageLocationOfVariable(_declaration);
 	}
 	else

libsolidity/ExpressionCompiler.h

@@ -119,7 +119,7 @@ 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);
-		void reset() { m_type = NONE; m_baseStackOffset = 0; }
+		void reset() { m_type = NONE; m_baseStackOffset = 0; m_size = 0; }
 
 		bool isValid() const { return m_type != NONE; }
 		bool isInOnStack() const { return m_type == STACK; }
@@ -138,7 +138,9 @@ private:
 		/// Also removes the stored value from the stack if @a _move is
 		/// true. @a _expression is the current expression, used for error reporting.
 		void storeValue(Expression const& _expression, bool _move = false) const;
-
+		/// Stores zero in the lvalue.
+		/// @a _expression is the current expression, used for error reporting.
+		void setToZero(Expression const& _expression) const;
 		/// Convenience function to convert the stored reference to a value and reset type to NONE if
 		/// the reference was not requested by @a _expression.
 		void retrieveValueIfLValueNotRequested(Expression const& _expression);
@@ -149,8 +151,8 @@ private:
 		/// If m_type is STACK, this is base stack offset (@see
 		/// CompilerContext::getBaseStackOffsetOfVariable) of a local variable.
 		unsigned m_baseStackOffset = 0;
-		/// Size of the value of this lvalue on the stack.
-		unsigned m_stackSize = 0;
+		/// Size of the value of this lvalue on the stack or the storage.
+		unsigned m_size = 0;
 	};
 
 	bool m_optimize;

libsolidity/Types.cpp

@@ -147,7 +147,7 @@ TypePointer IntegerType::unaryOperatorResult(Token::Value _operator) const
 {
 	// "delete" is ok for all integer types
 	if (_operator == Token::DELETE)
-		return shared_from_this();
+		return make_shared<VoidType>();
 	// no further unary operators for addresses
 	else if (isAddress())
 		return TypePointer();
@@ -408,6 +408,13 @@ u256 BoolType::literalValue(Literal const* _literal) const
 		BOOST_THROW_EXCEPTION(InternalCompilerError() << errinfo_comment("Bool type constructed from non-boolean literal."));
 }
 
+TypePointer BoolType::unaryOperatorResult(Token::Value _operator) const
+{
+	if (_operator == Token::DELETE)
+		return make_shared<VoidType>();
+	return (_operator == Token::NOT) ? shared_from_this() : TypePointer();
+}
+
 TypePointer BoolType::binaryOperatorResult(Token::Value _operator, TypePointer const& _other) const
 {
 	if (getCategory() != _other->getCategory())
@@ -432,6 +439,11 @@ bool ContractType::isExplicitlyConvertibleTo(Type const& _convertTo) const
 	return isImplicitlyConvertibleTo(_convertTo) || _convertTo.getCategory() == Category::INTEGER;
 }
 
+TypePointer ContractType::unaryOperatorResult(Token::Value _operator) const
+{
+	return _operator == Token::DELETE ? make_shared<VoidType>() : TypePointer();
+}
+
 bool ContractType::operator==(Type const& _other) const
 {
 	if (_other.getCategory() != getCategory())
@@ -440,14 +452,6 @@ bool ContractType::operator==(Type const& _other) const
 	return other.m_contract == m_contract;
 }
 
-u256 ContractType::getStorageSize() const
-{
-	u256 size = 0;
-	for (ASTPointer<VariableDeclaration> const& variable: m_contract.getStateVariables())
-		size += variable->getType()->getStorageSize();
-	return max<u256>(1, size);
-}
-
 string ContractType::toString() const
 {
 	return "contract " + m_contract.getName();
@@ -491,6 +495,11 @@ u256 ContractType::getFunctionIdentifier(string const& _functionName) const
 	return Invalid256;
 }
 
+TypePointer StructType::unaryOperatorResult(Token::Value _operator) const
+{
+	return _operator == Token::DELETE ? make_shared<VoidType>() : TypePointer();
+}
+
 bool StructType::operator==(Type const& _other) const
 {
 	if (_other.getCategory() != getCategory())

libsolidity/Types.h

@@ -259,10 +259,7 @@ public:
 	BoolType() {}
 	virtual Category getCategory() const { return Category::BOOL; }
 	virtual bool isExplicitlyConvertibleTo(Type const& _convertTo) const override;
-	virtual TypePointer unaryOperatorResult(Token::Value _operator) const override
-	{
-		return (_operator == Token::NOT || _operator == Token::DELETE) ? shared_from_this() : TypePointer();
-	}
+	virtual TypePointer unaryOperatorResult(Token::Value _operator) const override;
 	virtual TypePointer binaryOperatorResult(Token::Value _operator, TypePointer const& _other) const override;
 
 	virtual unsigned getCalldataEncodedSize() const { return 1; }
@@ -284,8 +281,8 @@ public:
 	virtual bool isImplicitlyConvertibleTo(Type const& _convertTo) const override;
 	/// Contracts can be converted to themselves and to integers.
 	virtual bool isExplicitlyConvertibleTo(Type const& _convertTo) const override;
+	virtual TypePointer unaryOperatorResult(Token::Value _operator) const override;
 	virtual bool operator==(Type const& _other) const override;
-	virtual u256 getStorageSize() const override;
 	virtual bool isValueType() const override { return true; }
 	virtual std::string toString() const override;
 
@@ -315,11 +312,7 @@ class StructType: public Type
 public:
 	virtual Category getCategory() const override { return Category::STRUCT; }
 	StructType(StructDefinition const& _struct): m_struct(_struct) {}
-	virtual TypePointer unaryOperatorResult(Token::Value _operator) const override
-	{
-		return _operator == Token::DELETE ? shared_from_this() : TypePointer();
-	}
-
+	virtual TypePointer unaryOperatorResult(Token::Value _operator) const override;
 	virtual bool operator==(Type const& _other) const override;
 	virtual u256 getStorageSize() const override;
 	virtual bool canLiveOutsideStorage() const override;

test/SolidityEndToEndTest.cpp

@@ -772,6 +772,67 @@ BOOST_AUTO_TEST_CASE(struct_reference)
 	BOOST_CHECK(callContractFunction("check()") == encodeArgs(true));
 }
 
+BOOST_AUTO_TEST_CASE(deleteStruct)
+{
+	char const* sourceCode = R"(
+		contract test {
+			struct topStruct {
+				nestedStruct nstr;
+				emptyStruct empty;
+				uint topValue;
+				mapping (uint => uint) topMapping;
+			}
+			uint toDelete;
+			topStruct str;
+			struct nestedStruct {
+				uint nestedValue;
+				mapping (uint => bool) nestedMapping;
+			}
+			struct emptyStruct{
+			}
+			function test(){
+				toDelete = 5;
+				str.topValue = 1;
+				str.topMapping[0] = 1;
+				str.topMapping[1] = 2;
+
+				str.nstr.nestedValue = 2;
+				str.nstr.nestedMapping[0] = true;
+				str.nstr.nestedMapping[1] = false;
+				uint v = 5;
+				delete v;
+				delete str;
+				delete toDelete;
+			}
+			function getToDelete() returns (uint res){
+				res = toDelete;
+			}
+			function getTopValue() returns(uint topValue){
+				topValue = str.topValue;
+			}
+			function getNestedValue() returns(uint nestedValue){
+				nestedValue = str.nstr.nestedValue;
+			}
+			function getTopMapping(uint index) returns(uint ret) {
+			   ret = str.topMapping[index];
+			}
+			function getNestedMapping(uint index) returns(bool ret) {
+				return str.nstr.nestedMapping[index];
+			}
+		})";
+
+	compileAndRun(sourceCode);
+
+	BOOST_CHECK(callContractFunction("getToDelete()") == encodeArgs(0));
+	BOOST_CHECK(callContractFunction("getTopValue()") == encodeArgs(0));
+	BOOST_CHECK(callContractFunction("getNestedValue()") == encodeArgs(0));
+	// mapping values should be the same
+	BOOST_CHECK(callContractFunction("getTopMapping(uint256)", 0) == encodeArgs(1));
+	BOOST_CHECK(callContractFunction("getTopMapping(uint256)", 1) == encodeArgs(2));
+	BOOST_CHECK(callContractFunction("getNestedMapping(uint256)", 0) == encodeArgs(true));
+	BOOST_CHECK(callContractFunction("getNestedMapping(uint256)", 1) == encodeArgs(false));
+}
+
 BOOST_AUTO_TEST_CASE(constructor)
 {
 	char const* sourceCode = "contract test {\n"
@@ -1243,6 +1304,7 @@ BOOST_AUTO_TEST_CASE(constructor_arguments)
 		contract Helper {
 			string3 name;
 			bool flag;
+
 			function Helper(string3 x, bool f) {
 				name = x;
 				flag = f;