Merge pull request #889 from LefterisJP/sol_ExprCompilerEncapsulation

No longer exposing retrieveValueFromStorage() as a public function

3 files changed, 21 insertions, 15 deletions

diff --git a/BaseTypes.h b/BaseTypes.h
index a8fd77c8..057289ef 100644
--- a/BaseTypes.h
+++ b/BaseTypes.h
@@ -41,6 +41,8 @@ struct Location
 		start(_start), end(_end), sourceName(_sourceName) { }
 	Location(): start(-1), end(-1) { }
 
+	bool isEmpty() const { return start == -1 && end == -1; }
+
 	int start;
 	int end;
 	std::shared_ptr<std::string const> sourceName;
@@ -49,6 +51,8 @@ struct Location
 /// Stream output for Location (used e.g. in boost exceptions).
 inline std::ostream& operator<<(std::ostream& _out, Location const& _location)
 {
+	if (_location.isEmpty())
+		return _out << "NO_LOCATION_SPECIFIED";
 	return _out << *_location.sourceName << "[" << _location.start << "," << _location.end << ")";
 }
 
diff --git a/ExpressionCompiler.cpp b/ExpressionCompiler.cpp
index bcd90acf..5d44c86f 100644
--- a/ExpressionCompiler.cpp
+++ b/ExpressionCompiler.cpp
@@ -66,7 +66,7 @@ bool ExpressionCompiler::visit(Assignment const& _assignment)
 	{
 		if (m_currentLValue.storesReferenceOnStack())
 			m_context << eth::Instruction::SWAP1 << eth::Instruction::DUP2;
-		m_currentLValue.retrieveValue(_assignment, true);
+		m_currentLValue.retrieveValue(_assignment.getType(), _assignment.getLocation(), true);
 		appendOrdinaryBinaryOperatorCode(Token::AssignmentToBinaryOp(op), *_assignment.getType());
 		if (m_currentLValue.storesReferenceOnStack())
 			m_context << eth::Instruction::SWAP1;
@@ -107,7 +107,7 @@ bool ExpressionCompiler::visit(UnaryOperation const& _unaryOperation)
 	case Token::INC: // ++ (pre- or postfix)
 	case Token::DEC: // -- (pre- or postfix)
 		solAssert(m_currentLValue.isValid(), "LValue not retrieved.");
-		m_currentLValue.retrieveValue(_unaryOperation);
+		m_currentLValue.retrieveValue(_unaryOperation.getType(), _unaryOperation.getLocation());
 		if (!_unaryOperation.isPrefixOperation())
 		{
 			if (m_currentLValue.storesReferenceOnStack())
@@ -811,7 +811,7 @@ void ExpressionCompiler::appendStateVariableAccessor(VariableDeclaration const&
 {
 	m_currentLValue.fromStateVariable(_varDecl, _varDecl.getType());
 	solAssert(m_currentLValue.isInStorage(), "");
-	m_currentLValue.retrieveValueFromStorage(_varDecl.getType(), true);
+	m_currentLValue.retrieveValue(_varDecl.getType(), Location(), true);
 }
 
 ExpressionCompiler::LValue::LValue(CompilerContext& _compilerContext, LValueType _type, Type const& _dataType,
@@ -826,7 +826,7 @@ ExpressionCompiler::LValue::LValue(CompilerContext& _compilerContext, LValueType
 		m_size = unsigned(_dataType.getSizeOnStack());
 }
 
-void ExpressionCompiler::LValue::retrieveValue(Expression const& _expression, bool _remove) const
+void ExpressionCompiler::LValue::retrieveValue(TypePointer const& _type, Location const& _location, bool _remove) const
 {
 	switch (m_type)
 	{
@@ -834,23 +834,23 @@ void ExpressionCompiler::LValue::retrieveValue(Expression const& _expression, bo
 	{
 		unsigned stackPos = m_context->baseToCurrentStackOffset(unsigned(m_baseStackOffset));
 		if (stackPos >= 15) //@todo correct this by fetching earlier or moving to memory
-			BOOST_THROW_EXCEPTION(CompilerError() << errinfo_sourceLocation(_expression.getLocation())
+			BOOST_THROW_EXCEPTION(CompilerError() << errinfo_sourceLocation(_location)
 												  << errinfo_comment("Stack too deep."));
 		for (unsigned i = 0; i < m_size; ++i)
 			*m_context << eth::dupInstruction(stackPos + 1);
 		break;
 	}
 	case STORAGE:
-		retrieveValueFromStorage(_expression.getType(), _remove);
+		retrieveValueFromStorage(_type, _remove);
 		break;
 	case MEMORY:
-		if (!_expression.getType()->isValueType())
+		if (!_type->isValueType())
 			break; // no distinction between value and reference for non-value types
-		BOOST_THROW_EXCEPTION(InternalCompilerError() << errinfo_sourceLocation(_expression.getLocation())
+		BOOST_THROW_EXCEPTION(InternalCompilerError() << errinfo_sourceLocation(_location)
 													  << errinfo_comment("Location type not yet implemented."));
 		break;
 	default:
-		BOOST_THROW_EXCEPTION(InternalCompilerError() << errinfo_sourceLocation(_expression.getLocation())
+		BOOST_THROW_EXCEPTION(InternalCompilerError() << errinfo_sourceLocation(_location)
 													  << errinfo_comment("Unsupported location type."));
 		break;
 	}
@@ -889,7 +889,7 @@ void ExpressionCompiler::LValue::storeValue(Expression const& _expression, bool
 			for (unsigned i = 0; i < m_size; ++i)
 				*m_context << eth::swapInstruction(stackDiff) << eth::Instruction::POP;
 		if (!_move)
-			retrieveValue(_expression);
+			retrieveValue(_expression.getType(), _expression.getLocation());
 		break;
 	}
 	case LValue::STORAGE:
@@ -976,7 +976,7 @@ void ExpressionCompiler::LValue::retrieveValueIfLValueNotRequested(Expression co
 {
 	if (!_expression.lvalueRequested())
 	{
-		retrieveValue(_expression, true);
+		retrieveValue(_expression.getType(), _expression.getLocation(), true);
 		reset();
 	}
 }
diff --git a/ExpressionCompiler.h b/ExpressionCompiler.h
index b4a64594..748cc6c6 100644
--- a/ExpressionCompiler.h
+++ b/ExpressionCompiler.h
@@ -130,10 +130,9 @@ private:
 
 		/// Copies the value of the current lvalue to the top of the stack and, if @a _remove is true,
 		/// 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 retrieve Value from Storage. Specific version of @ref retrieveValue
-		void retrieveValueFromStorage(TypePointer const& _type, bool _remove = false) const;
+		/// @a _type is the type of the current expression and @ _location its location, used for error reporting.
+		/// @a _location can be a nullptr for expressions that don't have an actual ASTNode equivalent
+		void retrieveValue(TypePointer const& _type, Location const& _location, 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
@@ -147,6 +146,9 @@ private:
 		void retrieveValueIfLValueNotRequested(Expression const& _expression);
 
 	private:
+		/// Convenience function to retrieve Value from Storage. Specific version of @ref retrieveValue
+		void retrieveValueFromStorage(TypePointer const& _type, bool _remove = false) const;
+
 		CompilerContext* m_context;
 		LValueType m_type = NONE;
 		/// If m_type is STACK, this is base stack offset (@see