16 files changed, 247 insertions, 41 deletions

diff --git a/Changelog.md b/Changelog.md
index 856ea1ca..1cc2ee4a 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -80,6 +80,8 @@ Bugfixes:
  * Fix NatSpec json output for `@notice` and `@dev` tags on contract definitions.
  * References Resolver: Enforce ``storage`` as data location for mappings.
  * References Resolver: Report error instead of assertion fail when FunctionType has an undeclared type as parameter.
+ * Type Checker: Disallow assignments to mappings within tuple assignments as well.
+ * Type Checker: Allow assignments to local variables of mapping types.
  * Type Checker: Consider fixed size arrays when checking for recursive structs.
  * Type Checker: Report error when using structs in events without experimental ABIEncoderV2. This used to crash or log the wrong values.
  * Type System: Allow arbitrary exponents for literals with a mantissa of zero.
diff --git a/libsolidity/analysis/NameAndTypeResolver.cpp b/libsolidity/analysis/NameAndTypeResolver.cpp
index 823378c7..c5ed079d 100644
--- a/libsolidity/analysis/NameAndTypeResolver.cpp
+++ b/libsolidity/analysis/NameAndTypeResolver.cpp
@@ -231,7 +231,7 @@ vector<Declaration const*> NameAndTypeResolver::cleanedDeclarations(
 				shared_ptr<FunctionType const> newFunctionType { d->functionType(false) };
 				if (!newFunctionType)
 					newFunctionType = d->functionType(true);
-				return newFunctionType && functionType->hasEqualArgumentTypes(*newFunctionType);
+				return newFunctionType && functionType->hasEqualParameterTypes(*newFunctionType);
 			}
 		))
 			uniqueFunctions.push_back(declaration);
diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp
index af4d44a6..7cec7c43 100644
--- a/libsolidity/analysis/TypeChecker.cpp
+++ b/libsolidity/analysis/TypeChecker.cpp
@@ -214,7 +214,7 @@ void TypeChecker::findDuplicateDefinitions(map<string, vector<T>> const& _defini
 			SecondarySourceLocation ssl;
 
 			for (size_t j = i + 1; j < overloads.size(); ++j)
-				if (FunctionType(*overloads[i]).hasEqualArgumentTypes(FunctionType(*overloads[j])))
+				if (FunctionType(*overloads[i]).hasEqualParameterTypes(FunctionType(*overloads[j])))
 				{
 					ssl.append("Other declaration is here:", overloads[j]->location());
 					reported.insert(j);
@@ -252,7 +252,7 @@ void TypeChecker::checkContractAbstractFunctions(ContractDefinition const& _cont
 			FunctionTypePointer funType = make_shared<FunctionType>(*function);
 			auto it = find_if(overloads.begin(), overloads.end(), [&](FunTypeAndFlag const& _funAndFlag)
 			{
-				return funType->hasEqualArgumentTypes(*_funAndFlag.first);
+				return funType->hasEqualParameterTypes(*_funAndFlag.first);
 			});
 			if (it == overloads.end())
 				overloads.push_back(make_pair(funType, function->isImplemented()));
@@ -404,7 +404,7 @@ void TypeChecker::checkFunctionOverride(FunctionDefinition const& function, Func
 	FunctionType functionType(function);
 	FunctionType superType(super);
 
-	if (!functionType.hasEqualArgumentTypes(superType))
+	if (!functionType.hasEqualParameterTypes(superType))
 		return;
 
 	if (!function.annotation().superFunction)
@@ -475,7 +475,7 @@ void TypeChecker::checkContractExternalTypeClashes(ContractDefinition const& _co
 	for (auto const& it: externalDeclarations)
 		for (size_t i = 0; i < it.second.size(); ++i)
 			for (size_t j = i + 1; j < it.second.size(); ++j)
-				if (!it.second[i].second->hasEqualArgumentTypes(*it.second[j].second))
+				if (!it.second[i].second->hasEqualParameterTypes(*it.second[j].second))
 					m_errorReporter.typeError(
 						it.second[j].first->location(),
 						"Function overload clash during conversion to external types for arguments."
@@ -1330,11 +1330,41 @@ bool TypeChecker::visit(Conditional const& _conditional)
 	return false;
 }
 
+void TypeChecker::checkExpressionAssignment(Type const& _type, Expression const& _expression)
+{
+	if (auto const* tupleExpression = dynamic_cast<TupleExpression const*>(&_expression))
+	{
+		auto const* tupleType = dynamic_cast<TupleType const*>(&_type);
+		auto const& types = tupleType ? tupleType->components() : vector<TypePointer> { _type.shared_from_this() };
+
+		solAssert(tupleExpression->components().size() == types.size(), "");
+		for (size_t i = 0; i < types.size(); i++)
+			if (types[i])
+			{
+				solAssert(!!tupleExpression->components()[i], "");
+				checkExpressionAssignment(*types[i], *tupleExpression->components()[i]);
+			}
+	}
+	else if (_type.category() == Type::Category::Mapping)
+	{
+		bool isLocalOrReturn = false;
+		if (auto const* identifier = dynamic_cast<Identifier const*>(&_expression))
+			if (auto const *variableDeclaration = dynamic_cast<VariableDeclaration const*>(identifier->annotation().referencedDeclaration))
+				if (variableDeclaration->isLocalOrReturn())
+					isLocalOrReturn = true;
+		if (!isLocalOrReturn)
+			m_errorReporter.typeError(_expression.location(), "Mappings cannot be assigned to.");
+	}
+}
+
 bool TypeChecker::visit(Assignment const& _assignment)
 {
 	requireLValue(_assignment.leftHandSide());
 	TypePointer t = type(_assignment.leftHandSide());
 	_assignment.annotation().type = t;
+
+	checkExpressionAssignment(*t, _assignment.leftHandSide());
+
 	if (TupleType const* tupleType = dynamic_cast<TupleType const*>(t.get()))
 	{
 		if (_assignment.assignmentOperator() != Token::Assign)
@@ -1351,11 +1381,6 @@ bool TypeChecker::visit(Assignment const& _assignment)
 		if (dynamic_cast<TupleType const*>(type(_assignment.rightHandSide()).get()))
 			checkDoubleStorageAssignment(_assignment);
 	}
-	else if (t->category() == Type::Category::Mapping)
-	{
-		m_errorReporter.typeError(_assignment.location(), "Mappings cannot be assigned to.");
-		_assignment.rightHandSide().accept(*this);
-	}
 	else if (_assignment.assignmentOperator() == Token::Assign)
 		expectType(_assignment.rightHandSide(), *t);
 	else
diff --git a/libsolidity/analysis/TypeChecker.h b/libsolidity/analysis/TypeChecker.h
index 8dc6b376..47892a3f 100644
--- a/libsolidity/analysis/TypeChecker.h
+++ b/libsolidity/analysis/TypeChecker.h
@@ -87,6 +87,9 @@ private:
 	/// Checks (and warns) if a tuple assignment might cause unexpected overwrites in storage.
 	/// Should only be called if the left hand side is tuple-typed.
 	void checkDoubleStorageAssignment(Assignment const& _assignment);
+	// Checks whether the expression @arg _expression can be assigned from type @arg _type
+	// and reports an error, if not.
+	void checkExpressionAssignment(Type const& _type, Expression const& _expression);
 
 	virtual void endVisit(InheritanceSpecifier const& _inheritance) override;
 	virtual void endVisit(UsingForDirective const& _usingFor) override;
diff --git a/libsolidity/ast/Types.cpp b/libsolidity/ast/Types.cpp
index 0954fdf3..7bcae812 100644
--- a/libsolidity/ast/Types.cpp
+++ b/libsolidity/ast/Types.cpp
@@ -1871,7 +1871,7 @@ MemberList::MemberMap ContractType::nativeMembers(ContractDefinition const* _con
 						continue;
 					auto memberType = dynamic_cast<FunctionType const*>(member.type.get());
 					solAssert(!!memberType, "Override changes type.");
-					if (!memberType->hasEqualArgumentTypes(*functionType))
+					if (!memberType->hasEqualParameterTypes(*functionType))
 						continue;
 					functionWithEqualArgumentsFound = true;
 					break;
@@ -2825,7 +2825,7 @@ bool FunctionType::canTakeArguments(TypePointers const& _argumentTypes, TypePoin
 		);
 }
 
-bool FunctionType::hasEqualArgumentTypes(FunctionType const& _other) const
+bool FunctionType::hasEqualParameterTypes(FunctionType const& _other) const
 {
 	if (m_parameterTypes.size() != _other.m_parameterTypes.size())
 		return false;
diff --git a/libsolidity/ast/Types.h b/libsolidity/ast/Types.h
index 12029a4e..89b8f170 100644
--- a/libsolidity/ast/Types.h
+++ b/libsolidity/ast/Types.h
@@ -1035,7 +1035,7 @@ public:
 	/// expression the function is called on.
 	bool canTakeArguments(TypePointers const& _arguments, TypePointer const& _selfType = TypePointer()) const;
 	/// @returns true if the types of parameters are equal (doesn't check return parameter types)
-	bool hasEqualArgumentTypes(FunctionType const& _other) const;
+	bool hasEqualParameterTypes(FunctionType const& _other) const;
 
 	/// @returns true if the ABI is used for this call (only meaningful for external calls)
 	bool isBareCall() const;
diff --git a/libsolidity/codegen/CompilerContext.cpp b/libsolidity/codegen/CompilerContext.cpp
index 3b1b4ec0..71b615b8 100644
--- a/libsolidity/codegen/CompilerContext.cpp
+++ b/libsolidity/codegen/CompilerContext.cpp
@@ -411,7 +411,7 @@ FunctionDefinition const& CompilerContext::resolveVirtualFunction(
 			if (
 				function->name() == name &&
 				!function->isConstructor() &&
-				FunctionType(*function).hasEqualArgumentTypes(functionType)
+				FunctionType(*function).hasEqualParameterTypes(functionType)
 			)
 				return *function;
 	solAssert(false, "Super function " + name + " not found.");
diff --git a/scripts/tests.sh b/scripts/tests.sh
index 0a1fcdac..1c0fc590 100755
--- a/scripts/tests.sh
+++ b/scripts/tests.sh
@@ -30,7 +30,11 @@ set -e
 
 REPO_ROOT="$(dirname "$0")"/..
 
+WORKDIR=`mktemp -d`
 IPC_ENABLED=true
+ALETH_PID=
+CMDLINE_PID=
+
 if [[ "$OSTYPE" == "darwin"* ]]
 then
     SMT_FLAGS="--no-smt"
@@ -41,6 +45,49 @@ then
     fi
 fi
 
+safe_kill() {
+    local PID=${1}
+    local NAME=${2:-${1}}
+    local n=1
+
+    # only proceed if $PID does exist
+    kill -0 $PID 2>/dev/null || return
+
+    echo "Sending SIGTERM to ${NAME} (${PID}) ..."
+    kill $PID
+
+    # wait until process terminated gracefully
+    while kill -0 $PID 2>/dev/null && [[ $n -le 4 ]]; do
+        echo "Waiting ($n) ..."
+        sleep 1
+        n=$[n + 1]
+    done
+
+    # process still alive? then hard-kill
+    if kill -0 $PID 2>/dev/null; then
+        echo "Sending SIGKILL to ${NAME} (${PID}) ..."
+        kill -9 $PID
+    fi
+}
+
+cleanup() {
+	# ensure failing commands don't cause termination during cleanup (especially within safe_kill)
+	set +e
+
+    if [[ "$IPC_ENABLED" = true ]] && [[ -n "${ALETH_PID}" ]]
+    then
+        safe_kill $ALETH_PID $ALETH_PATH
+    fi
+    if [[ -n "$CMDLINE_PID" ]]
+    then
+        safe_kill $CMDLINE_PID "Commandline tests"
+    fi
+
+    echo "Cleaning up working directory ${WORKDIR} ..."
+    rm -rf "$WORKDIR" || true
+}
+trap cleanup INT TERM
+
 if [ "$1" = --junit_report ]
 then
     if [ -z "$2" ]
@@ -57,12 +104,13 @@ function printError() { echo "$(tput setaf 1)$1$(tput sgr0)"; }
 function printTask() { echo "$(tput bold)$(tput setaf 2)$1$(tput sgr0)"; }
 
 printTask "Running commandline tests..."
-"$REPO_ROOT/test/cmdlineTests.sh" &
-CMDLINE_PID=$!
 # Only run in parallel if this is run on CI infrastructure
-if [ -z "$CI" ]
+if [[ -n "$CI" ]]
 then
-    if ! wait $CMDLINE_PID
+    "$REPO_ROOT/test/cmdlineTests.sh" &
+    CMDLINE_PID=$!
+else
+    if ! $REPO_ROOT/test/cmdlineTests.sh
     then
         printError "Commandline tests FAILED"
         exit 1
@@ -102,10 +150,10 @@ function download_aleth()
 # echos the PID
 function run_aleth()
 {
-    $ALETH_PATH --test -d "$1" >/dev/null 2>&1 &
+    $ALETH_PATH --test -d "${WORKDIR}" >/dev/null 2>&1 &
     echo $!
     # Wait until the IPC endpoint is available.
-    while [ ! -S "$1"/geth.ipc ] ; do sleep 1; done
+    while [ ! -S "${WORKDIR}/geth.ipc" ] ; do sleep 1; done
     sleep 2
 }
 
@@ -121,7 +169,7 @@ if [ "$IPC_ENABLED" = true ];
 then
     download_aleth
     check_aleth
-    ALETH_PID=$(run_aleth /tmp/test)
+    ALETH_PID=$(run_aleth)
 fi
 
 progress="--show-progress"
@@ -154,19 +202,15 @@ do
         log=--logger=JUNIT,test_suite,$log_directory/noopt_$vm.xml $testargs_no_opt
       fi
     fi
-    "$REPO_ROOT"/build/test/soltest $progress $log -- --testpath "$REPO_ROOT"/test "$optimize" --evm-version "$vm" $SMT_FLAGS $IPC_FLAGS  --ipcpath /tmp/test/geth.ipc
+    "$REPO_ROOT"/build/test/soltest $progress $log -- --testpath "$REPO_ROOT"/test "$optimize" --evm-version "$vm" $SMT_FLAGS $IPC_FLAGS  --ipcpath "${WORKDIR}/geth.ipc"
   done
 done
 
-if ! wait $CMDLINE_PID
+if [[ -n $CMDLINE_PID ]] && ! wait $CMDLINE_PID
 then
     printError "Commandline tests FAILED"
+    CMDLINE_PID=
     exit 1
 fi
 
-if [ "$IPC_ENABLED" = true ]
-then
-    pkill "$ALETH_PID" || true
-    sleep 4
-    pgrep "$ALETH_PID" && pkill -9 "$ALETH_PID" || true
-fi
+cleanup
diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp
index 45a56b22..066e97e6 100644
--- a/test/libsolidity/SolidityEndToEndTest.cpp
+++ b/test/libsolidity/SolidityEndToEndTest.cpp
@@ -1478,6 +1478,73 @@ BOOST_AUTO_TEST_CASE(multi_level_mapping)
 	testContractAgainstCpp("f(uint256,uint256,uint256)", f, u256(5), u256(4), u256(0));
 }
 
+BOOST_AUTO_TEST_CASE(mapping_local_assignment)
+{
+	char const* sourceCode = R"(
+		contract test {
+			mapping(uint8 => uint8) m1;
+			mapping(uint8 => uint8) m2;
+			function f() public returns (uint8, uint8, uint8, uint8) {
+				mapping(uint8 => uint8) storage m = m1;
+				m[1] = 42;
+
+				m = m2;
+				m[2] = 21;
+
+				return (m1[1], m1[2], m2[1], m2[2]);
+			}
+		}
+	)";
+	compileAndRun(sourceCode);
+
+	ABI_CHECK(callContractFunction("f()"), encodeArgs(byte(42), byte(0), byte(0), byte(21)));
+}
+
+BOOST_AUTO_TEST_CASE(mapping_local_tuple_assignment)
+{
+	char const* sourceCode = R"(
+		contract test {
+			mapping(uint8 => uint8) m1;
+			mapping(uint8 => uint8) m2;
+			function f() public returns (uint8, uint8, uint8, uint8) {
+				mapping(uint8 => uint8) storage m = m1;
+				m[1] = 42;
+
+				uint8 v;
+				(m, v) = (m2, 21);
+				m[2] = v;
+
+				return (m1[1], m1[2], m2[1], m2[2]);
+			}
+		}
+	)";
+	compileAndRun(sourceCode);
+
+	ABI_CHECK(callContractFunction("f()"), encodeArgs(byte(42), byte(0), byte(0), byte(21)));
+}
+
+BOOST_AUTO_TEST_CASE(mapping_local_compound_assignment)
+{
+	char const* sourceCode = R"(
+		contract test {
+			mapping(uint8 => uint8) m1;
+			mapping(uint8 => uint8) m2;
+			function f() public returns (uint8, uint8, uint8, uint8) {
+				mapping(uint8 => uint8) storage m = m1;
+				m[1] = 42;
+
+				(m = m2)[2] = 21;
+
+				return (m1[1], m1[2], m2[1], m2[2]);
+			}
+		}
+	)";
+	compileAndRun(sourceCode);
+
+	ABI_CHECK(callContractFunction("f()"), encodeArgs(byte(42), byte(0), byte(0), byte(21)));
+}
+
+
 BOOST_AUTO_TEST_CASE(structs)
 {
 	char const* sourceCode = R"(
diff --git a/test/libsolidity/syntaxTests/nameAndTypeResolution/016_assignment_to_mapping.sol b/test/libsolidity/syntaxTests/nameAndTypeResolution/016_assignment_to_mapping.sol
deleted file mode 100644
index 27b1ea96..00000000
--- a/test/libsolidity/syntaxTests/nameAndTypeResolution/016_assignment_to_mapping.sol
+++ /dev/null
@@ -1,12 +0,0 @@
-contract test {
-    struct str {
-        mapping(uint=>uint) map;
-    }
-    str data;
-    function fun() public {
-        mapping(uint=>uint) storage a = data.map;
-        data.map = a;
-    }
-}
-// ----
-// TypeError: (172-184): Mappings cannot be assigned to.
diff --git a/test/libsolidity/syntaxTests/types/mapping/assignment_local.sol b/test/libsolidity/syntaxTests/types/mapping/assignment_local.sol
new file mode 100644
index 00000000..a329c91e
--- /dev/null
+++ b/test/libsolidity/syntaxTests/types/mapping/assignment_local.sol
@@ -0,0 +1,11 @@
+contract test {
+    mapping(uint=>uint) map;
+    function fun() public view {
+        mapping(uint=>uint) storage a = map;
+        mapping(uint=>uint) storage b = map;
+        b = a;
+        (b) = a;
+        (b, b) = (a, a);
+    }
+}
+// ----
diff --git a/test/libsolidity/syntaxTests/types/mapping/assignment_state_variable.sol b/test/libsolidity/syntaxTests/types/mapping/assignment_state_variable.sol
new file mode 100644
index 00000000..1323afe6
--- /dev/null
+++ b/test/libsolidity/syntaxTests/types/mapping/assignment_state_variable.sol
@@ -0,0 +1,14 @@
+contract test {
+    mapping(uint=>uint) map;
+    function fun() public {
+        mapping(uint=>uint) storage a = map;
+        map = a;
+        (map) = a;
+        (map, map) = (a, a);
+    }
+}
+// ----
+// TypeError: (126-129): Mappings cannot be assigned to.
+// TypeError: (144-147): Mappings cannot be assigned to.
+// TypeError: (163-166): Mappings cannot be assigned to.
+// TypeError: (168-171): Mappings cannot be assigned to.
diff --git a/test/libsolidity/syntaxTests/types/mapping/assignment_struct.sol b/test/libsolidity/syntaxTests/types/mapping/assignment_struct.sol
new file mode 100644
index 00000000..b89241ed
--- /dev/null
+++ b/test/libsolidity/syntaxTests/types/mapping/assignment_struct.sol
@@ -0,0 +1,17 @@
+contract test {
+    struct str {
+        mapping(uint=>uint) map;
+    }
+    str data;
+    function fun() public {
+        mapping(uint=>uint) storage a = data.map;
+        data.map = a;
+        (data.map) = a;
+        (data.map, data.map) = (a, a);
+    }
+}
+// ----
+// TypeError: (172-180): Mappings cannot be assigned to.
+// TypeError: (195-203): Mappings cannot be assigned to.
+// TypeError: (219-227): Mappings cannot be assigned to.
+// TypeError: (229-237): Mappings cannot be assigned to.
diff --git a/test/libsolidity/syntaxTests/types/mapping/mapping_return_external.sol b/test/libsolidity/syntaxTests/types/mapping/mapping_return_external.sol
new file mode 100644
index 00000000..85121241
--- /dev/null
+++ b/test/libsolidity/syntaxTests/types/mapping/mapping_return_external.sol
@@ -0,0 +1,7 @@
+contract C {
+    function f() external pure returns (mapping(uint=>uint) storage m) {
+    }
+}
+// ----
+// TypeError: (53-82): Type is required to live outside storage.
+// TypeError: (53-82): Internal or recursive type is not allowed for public or external functions.
diff --git a/test/libsolidity/syntaxTests/types/mapping/mapping_return_internal.sol b/test/libsolidity/syntaxTests/types/mapping/mapping_return_internal.sol
new file mode 100644
index 00000000..a46003f8
--- /dev/null
+++ b/test/libsolidity/syntaxTests/types/mapping/mapping_return_internal.sol
@@ -0,0 +1,21 @@
+// This should be allowed in a future release.
+contract C {
+    mapping(uint=>uint) m;
+    function f() internal view returns (mapping(uint=>uint) storage) {
+        return m;
+    }
+    function g() private view returns (mapping(uint=>uint) storage) {
+        return m;
+    }
+    function h() internal view returns (mapping(uint=>uint) storage r) {
+        r = m;
+    }
+    function i() private view returns (mapping(uint=>uint) storage r) {
+        (r,r) = (m,m);
+    }
+}
+// ----
+// TypeError: (127-146): Type is required to live outside storage.
+// TypeError: (221-240): Type is required to live outside storage.
+// TypeError: (316-345): Type is required to live outside storage.
+// TypeError: (409-438): Type is required to live outside storage.
diff --git a/test/libsolidity/syntaxTests/types/mapping/mapping_return_public.sol b/test/libsolidity/syntaxTests/types/mapping/mapping_return_public.sol
new file mode 100644
index 00000000..383fa797
--- /dev/null
+++ b/test/libsolidity/syntaxTests/types/mapping/mapping_return_public.sol
@@ -0,0 +1,7 @@
+contract C {
+    function f() public pure returns (mapping(uint=>uint) storage m) {
+    }
+}
+// ----
+// TypeError: (51-80): Type is required to live outside storage.
+// TypeError: (51-80): Internal or recursive type is not allowed for public or external functions.