Allow calling internal functions of libraries.

Internal functions of libraries can be called as if the library were a
base contract of the calling contract. As the calling convention for
internal functions is to not create a new call context, the code of
these functions will be pulled into the context of the caller,
duplicating their code. This might pull in code of further internal or
even private functions.

The use case for such functions is to allow libraries which can operate
on memory types such that these types can also be modified in place.

5 files changed, 193 insertions, 59 deletions

diff --git a/libsolidity/ast/Types.cpp b/libsolidity/ast/Types.cpp
index 4dc1eb13..d541de23 100644
--- a/libsolidity/ast/Types.cpp
+++ b/libsolidity/ast/Types.cpp
@@ -197,7 +197,7 @@ MemberList const& Type::members(ContractDefinition const* _currentScope) const
 	{
 		MemberList::MemberMap members = nativeMembers(_currentScope);
 		if (_currentScope)
-			members +=  boundFunctions(*this, *_currentScope);
+			members += boundFunctions(*this, *_currentScope);
 		m_members[_currentScope] = unique_ptr<MemberList>(new MemberList(move(members)));
 	}
 	return *m_members[_currentScope];
@@ -220,19 +220,14 @@ MemberList::MemberMap Type::boundFunctions(Type const& _type, ContractDefinition
 			auto const& library = dynamic_cast<ContractDefinition const&>(
 				*ufd->libraryName().annotation().referencedDeclaration
 			);
-			for (auto const& it: library.interfaceFunctions())
+			for (FunctionDefinition const* function: library.definedFunctions())
 			{
-				FunctionType const& funType = *it.second;
-				solAssert(funType.hasDeclaration(), "Tried to bind function without declaration.");
-				if (seenFunctions.count(&funType.declaration()))
+				if (!function->isVisibleInDerivedContracts() || seenFunctions.count(function))
 					continue;
-				seenFunctions.insert(&funType.declaration());
+				seenFunctions.insert(function);
+				FunctionType funType(*function, false);
 				if (auto fun = funType.asMemberFunction(true, true))
-					members.push_back(MemberList::Member(
-						funType.declaration().name(),
-						fun,
-						&funType.declaration()
-					));
+					members.push_back(MemberList::Member(function->name(), fun, function));
 			}
 		}
 	return members;
@@ -1052,7 +1047,7 @@ MemberList::MemberMap ContractType::nativeMembers(ContractDefinition const*) con
 					));
 			}
 	}
-	else
+	else if (!m_contract.isLibrary())
 	{
 		for (auto const& it: m_contract.interfaceFunctions())
 			members.push_back(MemberList::Member(
@@ -1772,21 +1767,40 @@ FunctionTypePointer FunctionType::asMemberFunction(bool _inLibrary, bool _bound)
 			parameterTypes.push_back(t);
 	}
 
-	// Removes dynamic types.
+	Location location = m_location;
+	if (_inLibrary)
+	{
+		solAssert(!!m_declaration, "Declaration has to be available.");
+		if (!m_declaration->isPublic())
+			location = Location::Internal; // will be inlined
+		else
+			location = Location::DelegateCall;
+	}
+
 	TypePointers returnParameterTypes;
 	vector<string> returnParameterNames;
-	for (size_t i = 0; i < m_returnParameterTypes.size(); ++i)
-		if (!m_returnParameterTypes[i]->isDynamicallySized())
-		{
-			returnParameterTypes.push_back(m_returnParameterTypes[i]);
-			returnParameterNames.push_back(m_returnParameterNames[i]);
-		}
+	if (location == Location::Internal)
+	{
+		returnParameterNames = m_returnParameterNames;
+		returnParameterTypes = m_returnParameterTypes;
+	}
+	else
+	{
+		// Removes dynamic types.
+		for (size_t i = 0; i < m_returnParameterTypes.size(); ++i)
+			if (!m_returnParameterTypes[i]->isDynamicallySized())
+			{
+				returnParameterTypes.push_back(m_returnParameterTypes[i]);
+				returnParameterNames.push_back(m_returnParameterNames[i]);
+			}
+	}
+
 	return make_shared<FunctionType>(
 		parameterTypes,
 		returnParameterTypes,
 		m_parameterNames,
 		returnParameterNames,
-		_inLibrary ? Location::DelegateCall : m_location,
+		location,
 		m_arbitraryParameters,
 		m_declaration,
 		m_gasSet,
@@ -1882,12 +1896,13 @@ MemberList::MemberMap TypeType::nativeMembers(ContractDefinition const* _current
 			isBase = (find(currentBases.begin(), currentBases.end(), &contract) != currentBases.end());
 		}
 		if (contract.isLibrary())
-			for (auto const& it: contract.interfaceFunctions())
-				members.push_back(MemberList::Member(
-					it.second->declaration().name(),
-					it.second->asMemberFunction(true),
-					&it.second->declaration()
-				));
+			for (FunctionDefinition const* function: contract.definedFunctions())
+				if (function->isVisibleInDerivedContracts())
+					members.push_back(MemberList::Member(
+						function->name(),
+						FunctionType(*function).asMemberFunction(true),
+						function
+					));
 		if (isBase)
 		{
 			// We are accessing the type of a base contract, so add all public and protected
diff --git a/libsolidity/codegen/CompilerContext.cpp b/libsolidity/codegen/CompilerContext.cpp
index 53c2da05..c1dc8dfb 100644
--- a/libsolidity/codegen/CompilerContext.cpp
+++ b/libsolidity/codegen/CompilerContext.cpp
@@ -91,6 +91,12 @@ eth::AssemblyItem CompilerContext::functionEntryLabelIfExists(Declaration const&
 
 eth::AssemblyItem CompilerContext::virtualFunctionEntryLabel(FunctionDefinition const& _function)
 {
+	// Libraries do not allow inheritance and their functions can be inlined, so we should not
+	// search the inheritance hierarchy (which will be the wrong one in case the function
+	// is inlined).
+	if (auto scope = dynamic_cast<ContractDefinition const*>(_function.scope()))
+		if (scope->isLibrary())
+			return functionEntryLabel(_function);
 	solAssert(!m_inheritanceHierarchy.empty(), "No inheritance hierarchy set.");
 	return virtualFunctionEntryLabel(_function, m_inheritanceHierarchy.begin());
 }
diff --git a/libsolidity/codegen/ExpressionCompiler.cpp b/libsolidity/codegen/ExpressionCompiler.cpp
index d1cbc8ed..ab02e0b5 100644
--- a/libsolidity/codegen/ExpressionCompiler.cpp
+++ b/libsolidity/codegen/ExpressionCompiler.cpp
@@ -464,8 +464,8 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall)
 	{
 		FunctionType const& function = *functionType;
 		if (function.bound())
-			// Only delegatecall functions can be bound, this might be lifted later.
-			solAssert(function.location() == Location::DelegateCall, "");
+			// Only delegatecall and internal functions can be bound, this might be lifted later.
+			solAssert(function.location() == Location::DelegateCall || function.location() == Location::Internal, "");
 		switch (function.location())
 		{
 		case Location::Internal:
@@ -480,13 +480,21 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall)
 				utils().convertType(*arguments[i]->annotation().type, *function.parameterTypes()[i]);
 			}
 			_functionCall.expression().accept(*this);
+			unsigned parameterSize = CompilerUtils::sizeOnStack(function.parameterTypes());
+			if (function.bound())
+			{
+				// stack: arg2, ..., argn, label, arg1
+				unsigned depth = parameterSize + 1;
+				utils().moveIntoStack(depth, function.selfType()->sizeOnStack());
+				parameterSize += function.selfType()->sizeOnStack();
+			}
 
 			m_context.appendJump(eth::AssemblyItem::JumpType::IntoFunction);
 			m_context << returnLabel;
 
 			unsigned returnParametersSize = CompilerUtils::sizeOnStack(function.returnParameterTypes());
 			// callee adds return parameters, but removes arguments and return label
-			m_context.adjustStackOffset(returnParametersSize - CompilerUtils::sizeOnStack(function.parameterTypes()) - 1);
+			m_context.adjustStackOffset(returnParametersSize - parameterSize - 1);
 			break;
 		}
 		case Location::External:
@@ -809,7 +817,7 @@ bool ExpressionCompiler::visit(NewExpression const&)
 	return false;
 }
 
-void ExpressionCompiler::endVisit(MemberAccess const& _memberAccess)
+bool ExpressionCompiler::visit(MemberAccess const& _memberAccess)
 {
 	CompilerContext::LocationSetter locationSetter(m_context, _memberAccess);
 	// Check whether the member is a bound function.
@@ -817,19 +825,68 @@ void ExpressionCompiler::endVisit(MemberAccess const& _memberAccess)
 	if (auto funType = dynamic_cast<FunctionType const*>(_memberAccess.annotation().type.get()))
 		if (funType->bound())
 		{
+			_memberAccess.expression().accept(*this);
 			utils().convertType(
 				*_memberAccess.expression().annotation().type,
 				*funType->selfType(),
 				true
 			);
-			auto contract = dynamic_cast<ContractDefinition const*>(funType->declaration().scope());
-			solAssert(contract && contract->isLibrary(), "");
-			m_context.appendLibraryAddress(contract->name());
-			m_context << funType->externalIdentifier();
-			utils().moveIntoStack(funType->selfType()->sizeOnStack(), 2);
-			return;
+			if (funType->location() == FunctionType::Location::Internal)
+			{
+				m_context << m_context.functionEntryLabel(
+					dynamic_cast<FunctionDefinition const&>(funType->declaration())
+				).pushTag();
+				utils().moveIntoStack(funType->selfType()->sizeOnStack(), 1);
+			}
+			else
+			{
+				solAssert(funType->location() == FunctionType::Location::DelegateCall, "");
+				auto contract = dynamic_cast<ContractDefinition const*>(funType->declaration().scope());
+				solAssert(contract && contract->isLibrary(), "");
+				m_context.appendLibraryAddress(contract->name());
+				m_context << funType->externalIdentifier();
+				utils().moveIntoStack(funType->selfType()->sizeOnStack(), 2);
+			}
+			return false;
 		}
 
+	// Special processing for TypeType because we do not want to visit the library itself
+	// for internal functions.
+	if (TypeType const* type = dynamic_cast<TypeType const*>(_memberAccess.expression().annotation().type.get()))
+	{
+		if (dynamic_cast<ContractType const*>(type->actualType().get()))
+		{
+			if (auto funType = dynamic_cast<FunctionType const*>(_memberAccess.annotation().type.get()))
+			{
+				if (funType->location() != FunctionType::Location::Internal)
+				{
+					_memberAccess.expression().accept(*this);
+					m_context << funType->externalIdentifier();
+				}
+				else
+				{
+					// We do not visit the expression here on purpose, because in the case of an
+					// internal library function call, this would push the library address forcing
+					// us to link against it although we actually do not need it.
+					auto const* function = dynamic_cast<FunctionDefinition const*>(_memberAccess.annotation().referencedDeclaration);
+					solAssert(!!function, "Function not found in member access");
+					m_context << m_context.functionEntryLabel(*function).pushTag();
+				}
+			}
+			else
+				_memberAccess.expression().accept(*this);
+		}
+		else if (auto enumType = dynamic_cast<EnumType const*>(type->actualType().get()))
+		{
+			_memberAccess.expression().accept(*this);
+			m_context << enumType->memberValue(_memberAccess.memberName());
+		}
+		else
+			_memberAccess.expression().accept(*this);
+		return false;
+	}
+
+	_memberAccess.expression().accept(*this);
 	switch (_memberAccess.expression().annotation().type->category())
 	{
 	case Type::Category::Contract:
@@ -948,28 +1005,6 @@ void ExpressionCompiler::endVisit(MemberAccess const& _memberAccess)
 		m_context << type.memberValue(_memberAccess.memberName());
 		break;
 	}
-	case Type::Category::TypeType:
-	{
-		TypeType const& type = dynamic_cast<TypeType const&>(*_memberAccess.expression().annotation().type);
-
-		if (dynamic_cast<ContractType const*>(type.actualType().get()))
-		{
-			if (auto funType = dynamic_cast<FunctionType const*>(_memberAccess.annotation().type.get()))
-			{
-				if (funType->location() != FunctionType::Location::Internal)
-					m_context << funType->externalIdentifier();
-				else
-				{
-					auto const* function = dynamic_cast<FunctionDefinition const*>(_memberAccess.annotation().referencedDeclaration);
-					solAssert(!!function, "Function not found in member access");
-					m_context << m_context.functionEntryLabel(*function).pushTag();
-				}
-			}
-		}
-		else if (auto enumType = dynamic_cast<EnumType const*>(type.actualType().get()))
-			m_context << enumType->memberValue(_memberAccess.memberName());
-		break;
-	}
 	case Type::Category::Array:
 	{
 		auto const& type = dynamic_cast<ArrayType const&>(*_memberAccess.expression().annotation().type);
@@ -1018,6 +1053,7 @@ void ExpressionCompiler::endVisit(MemberAccess const& _memberAccess)
 	default:
 		BOOST_THROW_EXCEPTION(InternalCompilerError() << errinfo_comment("Member access to unknown type."));
 	}
+	return false;
 }
 
 bool ExpressionCompiler::visit(IndexAccess const& _indexAccess)
diff --git a/libsolidity/codegen/ExpressionCompiler.h b/libsolidity/codegen/ExpressionCompiler.h
index f00b24e8..43a92a10 100644
--- a/libsolidity/codegen/ExpressionCompiler.h
+++ b/libsolidity/codegen/ExpressionCompiler.h
@@ -78,7 +78,7 @@ private:
 	virtual bool visit(BinaryOperation const& _binaryOperation) override;
 	virtual bool visit(FunctionCall const& _functionCall) override;
 	virtual bool visit(NewExpression const& _newExpression) override;
-	virtual void endVisit(MemberAccess const& _memberAccess) override;
+	virtual bool visit(MemberAccess const& _memberAccess) override;
 	virtual bool visit(IndexAccess const& _indexAccess) override;
 	virtual void endVisit(Identifier const& _identifier) override;
 	virtual void endVisit(Literal const& _literal) override;
diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp
index f9d2ab2b..6233e25f 100644
--- a/test/libsolidity/SolidityEndToEndTest.cpp
+++ b/test/libsolidity/SolidityEndToEndTest.cpp
@@ -6633,6 +6633,83 @@ BOOST_AUTO_TEST_CASE(delete_on_array_of_structs)
 	BOOST_CHECK(callContractFunction("f()") == encodeArgs(true));
 }
 
+BOOST_AUTO_TEST_CASE(internal_library_function)
+{
+	// tests that internal library functions can be called from outside
+	// and retain the same memory context (i.e. are pulled into the caller's code)
+	char const* sourceCode = R"(
+		library L {
+			function f(uint[] _data) internal {
+				_data[3] = 2;
+			}
+		}
+		contract C {
+			function f() returns (uint) {
+				uint[] memory x = new uint[](7);
+				x[3] = 8;
+				L.f(x);
+				return x[3];
+			}
+		}
+	)";
+	// This has to work without linking, because everything will be inlined.
+	compileAndRun(sourceCode, 0, "C");
+	BOOST_CHECK(callContractFunction("f()") == encodeArgs(u256(2)));
+}
+
+BOOST_AUTO_TEST_CASE(internal_library_function_calling_private)
+{
+	// tests that internal library functions that are called from outside and that
+	// themselves call private functions are still able to (i.e. the private function
+	// also has to be pulled into the caller's code)
+	char const* sourceCode = R"(
+		library L {
+			function g(uint[] _data) private {
+				_data[3] = 2;
+			}
+			function f(uint[] _data) internal {
+				g(_data);
+			}
+		}
+		contract C {
+			function f() returns (uint) {
+				uint[] memory x = new uint[](7);
+				x[3] = 8;
+				L.f(x);
+				return x[3];
+			}
+		}
+	)";
+	// This has to work without linking, because everything will be inlined.
+	compileAndRun(sourceCode, 0, "C");
+	BOOST_CHECK(callContractFunction("f()") == encodeArgs(u256(2)));
+}
+
+BOOST_AUTO_TEST_CASE(internal_library_function_bound)
+{
+	char const* sourceCode = R"(
+		library L {
+			struct S { uint[] data; }
+			function f(S _s) internal {
+				_s.data[3] = 2;
+			}
+		}
+		contract C {
+			using L for L.S;
+			function f() returns (uint) {
+				L.S memory x;
+				x.data = new uint[](7);
+				x.data[3] = 8;
+				x.f();
+				return x.data[3];
+			}
+		}
+	)";
+	// This has to work without linking, because everything will be inlined.
+	compileAndRun(sourceCode, 0, "C");
+	BOOST_CHECK(callContractFunction("f()") == encodeArgs(u256(2)));
+}
+
 BOOST_AUTO_TEST_SUITE_END()
 
 }