Merge pull request #7221 from ethereum/calldataLValueError

Allow writing to value-type arguments in external functions + Better error messages on attempted writes to calldata.

Changelog.md

@@ -1,6 +1,7 @@
 ### 0.5.12 (unreleased)
 
 Language Features:
+ * Type Checker: Allow assignment to external function arguments except for reference types.
 
 
 Compiler Features:

libsolidity/analysis/TypeChecker.cpp

@@ -2515,8 +2515,47 @@ void TypeChecker::requireLValue(Expression const& _expression)
 	_expression.annotation().lValueRequested = true;
 	_expression.accept(*this);
 
-	if (_expression.annotation().isConstant)
+	if (_expression.annotation().isLValue)
-		m_errorReporter.typeError(_expression.location(), "Cannot assign to a constant variable.");
+		return;
-	else if (!_expression.annotation().isLValue)
+
-		m_errorReporter.typeError(_expression.location(), "Expression has to be an lvalue.");
+	return m_errorReporter.typeError(_expression.location(), [&]() {
+		if (_expression.annotation().isConstant)
+			return "Cannot assign to a constant variable.";
+
+		if (auto indexAccess = dynamic_cast<IndexAccess const*>(&_expression))
+		{
+			if (type(indexAccess->baseExpression())->category() == Type::Category::FixedBytes)
+				return "Single bytes in fixed bytes arrays cannot be modified.";
+			else if (auto arrayType = dynamic_cast<ArrayType const*>(type(indexAccess->baseExpression())))
+				if (arrayType->dataStoredIn(DataLocation::CallData))
+					return "Calldata arrays are read-only.";
+		}
+
+		if (auto memberAccess = dynamic_cast<MemberAccess const*>(&_expression))
+		{
+			if (auto structType = dynamic_cast<StructType const*>(type(memberAccess->expression())))
+			{
+				if (structType->dataStoredIn(DataLocation::CallData))
+					return "Calldata structs are read-only.";
+			}
+			else if (auto arrayType = dynamic_cast<ArrayType const*>(type(memberAccess->expression())))
+				if (memberAccess->memberName() == "length")
+					switch (arrayType->location())
+					{
+						case DataLocation::Memory:
+							return "Memory arrays cannot be resized.";
+						case DataLocation::CallData:
+							return "Calldata arrays cannot be resized.";
+						case DataLocation::Storage:
+							break;
+					}
+		}
+
+		if (auto identifier = dynamic_cast<Identifier const*>(&_expression))
+			if (auto varDecl = dynamic_cast<VariableDeclaration const*>(identifier->annotation().referencedDeclaration))
+				if (varDecl->isExternalCallableParameter() && dynamic_cast<ReferenceType const*>(identifier->annotation().type))
+					return "External function arguments of reference type are read-only.";
+
+		return "Expression has to be an lvalue.";
+	}());
 }

libsolidity/ast/AST.cpp

@@ -432,8 +432,13 @@ string Scopable::sourceUnitName() const
 
 bool VariableDeclaration::isLValue() const
 {
-	// External function parameters and constant declared variables are Read-Only
+	// Constant declared variables are Read-Only
-	return !isExternalCallableParameter() && !m_isConstant;
+	if (m_isConstant)
+		return false;
+	// External function arguments of reference type are Read-Only
+	if (isExternalCallableParameter() && dynamic_cast<ReferenceType const*>(type()))
+		return false;
+	return true;
 }
 
 bool VariableDeclaration::isLocalVariable() const

test/libsolidity/semanticTests/types/assign_calldata_value_type.sol

@@ -0,0 +1,9 @@
+contract C {
+    function f(uint256 x) public pure returns (uint256, uint256) {
+        uint256 b = x;
+        x = 42;
+        return (x, b);
+    }
+}
+// ----
+// f(uint256): 23 -> 42, 23

test/libsolidity/syntaxTests/array/calldata_assign.sol

@@ -0,0 +1,7 @@
+pragma experimental ABIEncoderV2;
+contract Test {
+    function f(uint256[] calldata s) external { s[0] = 4; }
+}
+// ----
+// Warning: (0-33): Experimental features are turned on. Do not use experimental features on live deployments.
+// TypeError: (98-102): Calldata arrays are read-only.

test/libsolidity/syntaxTests/array/calldata_resize.sol

@@ -0,0 +1,7 @@
+contract C {
+    function f (uint256[] calldata x) external pure {
+        x.length = 42;
+    }
+}
+// ----
+// TypeError: (75-83): Calldata arrays cannot be resized.

test/libsolidity/syntaxTests/lvalues/calldata_index_access.sol

@@ -0,0 +1,7 @@
+contract C {
+    function f(uint256[] calldata x) external pure {
+        x[0] = 42;
+    }
+}
+// ----
+// TypeError: (74-78): Calldata arrays are read-only.

test/libsolidity/syntaxTests/lvalues/calldata_member_access.sol

@@ -0,0 +1,10 @@
+pragma experimental ABIEncoderV2;
+contract C {
+    struct S { uint256 x; }
+    function f(S calldata s) external pure {
+        s.x = 42;
+    }
+}
+// ----
+// Warning: (0-33): Experimental features are turned on. Do not use experimental features on live deployments.
+// TypeError: (128-131): Calldata structs are read-only.

test/libsolidity/syntaxTests/lvalues/external_reference_argument.sol

@@ -0,0 +1,7 @@
+contract C {
+    function f(uint256[] calldata x, uint256[] calldata y) external pure {
+        x = y;
+    }
+}
+// ----
+// TypeError: (96-97): External function arguments of reference type are read-only.

test/libsolidity/syntaxTests/lvalues/functions.sol

@@ -0,0 +1,15 @@
+contract C {
+    function f() internal {
+    }
+    function g() internal {
+        g = f;
+    }
+    function h() external {
+    }
+    function i() external {
+        this.i = this.h;
+    }
+}
+// ----
+// TypeError: (83-84): Expression has to be an lvalue.
+// TypeError: (166-172): Expression has to be an lvalue.

test/libsolidity/syntaxTests/lvalues/library_mapping.sol

@@ -0,0 +1,7 @@
+library L {
+    function f(mapping(uint=>uint) storage x, mapping(uint=>uint) storage y) external {
+        x = y;
+    }
+}
+// ----
+// TypeError: (108-109): Mappings cannot be assigned to.

test/libsolidity/syntaxTests/lvalues/valid_lvalues.sol

@@ -0,0 +1,31 @@
+contract C {
+    struct S { uint256 x; }
+	function i() internal pure {}
+	function e() external pure {}
+	uint[] s1;
+    function f(uint x, bytes32 y) external {
+        x = 42;
+        y = bytes32(0);
+        (x, y) = (23, bytes32(0));
+        S memory ms1;
+        S memory ms2;
+        ms1 = ms2;
+        ms1.x = x;
+        uint256[] memory a = new uint256[](2);
+        uint256[] memory b = new uint256[](3);
+        a = b;
+        a[0] = x;
+        s1[0] = x;
+        s1 = a;
+    }
+    function g(function() internal pure x) internal view {
+        x = i;
+        function(uint, bytes32) external y;
+        y = this.f;
+    }
+    function g(function() external pure x) external view {
+        x = this.e;
+        function(function() internal pure) internal view y;
+        y = g;
+    }
+}

test/libsolidity/syntaxTests/nameAndTypeResolution/146_external_argument_assign.sol

@@ -1,5 +1,4 @@
 contract c {
-    function f(uint a) external { a = 1; }
+    function f(uint a) external pure { a = 1; }
 }
 // ----
-// TypeError: (47-48): Expression has to be an lvalue.

test/libsolidity/syntaxTests/nameAndTypeResolution/147_external_argument_increment.sol

@@ -1,5 +1,4 @@
 contract c {
-    function f(uint a) external { a++; }
+    function f(uint a) external pure { a++; }
 }
 // ----
-// TypeError: (47-48): Expression has to be an lvalue.

test/libsolidity/syntaxTests/nameAndTypeResolution/148_external_argument_delete.sol

@@ -2,4 +2,4 @@ contract c {
     function f(uint a) external { delete a; }
 }
 // ----
-// TypeError: (54-55): Expression has to be an lvalue.
+// Warning: (17-58): Function state mutability can be restricted to pure

test/libsolidity/syntaxTests/nameAndTypeResolution/219_memory_arrays_not_resizeable.sol

@@ -5,4 +5,4 @@ contract C {
     }
 }
 // ----
-// TypeError: (72-80): Expression has to be an lvalue.
+// TypeError: (72-80): Memory arrays cannot be resized.

test/libsolidity/syntaxTests/structs/calldata_array_assign.sol

@@ -7,4 +7,4 @@ contract Test {
 }
 // ----
 // Warning: (0-33): Experimental features are turned on. Do not use experimental features on live deployments.
-// TypeError: (144-147): Expression has to be an lvalue.
+// TypeError: (144-147): Calldata structs are read-only.

test/libsolidity/syntaxTests/structs/calldata_assign.sol

@@ -5,4 +5,4 @@ contract Test {
 }
 // ----
 // Warning: (0-33): Experimental features are turned on. Do not use experimental features on live deployments.
-// TypeError: (114-117): Expression has to be an lvalue.
+// TypeError: (114-117): Calldata structs are read-only.

test/libsolidity/syntaxTests/structs/memory_to_calldata.sol

@@ -6,7 +6,7 @@ contract Test {
 }
 // ----
 // Warning: (0-33): Experimental features are turned on. Do not use experimental features on live deployments.
-// TypeError: (114-115): Expression has to be an lvalue.
+// TypeError: (114-115): External function arguments of reference type are read-only.
 // TypeError: (118-122): Type struct Test.S memory is not implicitly convertible to expected type struct Test.S calldata.
-// TypeError: (178-179): Expression has to be an lvalue.
+// TypeError: (178-179): External function arguments of reference type are read-only.
 // TypeError: (182-183): Type struct Test.S memory is not implicitly convertible to expected type struct Test.S calldata.

test/libsolidity/syntaxTests/types/bytesXX_index_assign.sol

@@ -0,0 +1,8 @@
+contract C {
+    function f() public pure {
+        bytes32 x;
+        x[0] = 0x42;
+    }
+}
+// ----
+// TypeError: (71-75): Single bytes in fixed bytes arrays cannot be modified.