Merge pull request #4527 from ethereum/mappingEnforceStorage

Enforces explicit data location for mappings

Changelog.md

@@ -72,6 +72,7 @@ Bugfixes:
  * Tests: Fix chain parameters to make ipc tests work with newer versions of cpp-ethereum.
  * Code Generator: Fix allocation of byte arrays (zeroed out too much memory).
  * Fix NatSpec json output for `@notice` and `@dev` tags on contract definitions.
+ * References Resolver: Enforce ``storage`` as data location for mappings.
  * Type Checker: Consider fixed size arrays when checking for recursive structs.
  * Type System: Allow arbitrary exponents for literals with a mantissa of zero.
 

libsolidity/analysis/ReferencesResolver.cpp

@@ -404,9 +404,16 @@ void ReferencesResolver::endVisit(VariableDeclaration const& _variable)
 				}
 				isPointer = !_variable.isStateVariable();
 			}
-
 			type = ref->copyForLocation(typeLoc, isPointer);
 		}
+		else if (dynamic_cast<MappingType const*>(type.get()))
+		{
+			if (_variable.isLocalVariable() && varLoc != Location::Storage)
+				typeError(
+					_variable.location(),
+					"Data location for mappings must be specified as \"storage\"."
+				);
+		}
 		else if (varLoc != Location::Default && !ref)
 			typeError(_variable.location(), "Data location can only be given for array or struct types.");
 

test/libsolidity/SolidityEndToEndTest.cpp

@@ -1534,7 +1534,7 @@ BOOST_AUTO_TEST_CASE(struct_reference)
 			}
 			function set() public {
 				data.z = 2;
-				mapping(uint8 => s2) map = data.recursive;
+				mapping(uint8 => s2) storage map = data.recursive;
 				s2 storage inner = map[0];
 				inner.z = 3;
 				inner.recursive[0].z = inner.recursive[1].z + 1;

test/libsolidity/syntaxTests/nameAndTypeResolution/016_assignment_to_mapping.sol

@@ -4,9 +4,9 @@ contract test {
     }
     str data;
     function fun() public {
-        mapping(uint=>uint) a = data.map;
+        mapping(uint=>uint) storage a = data.map;
         data.map = a;
     }
 }
 // ----
-// TypeError: (164-176): Mappings cannot be assigned to.
+// TypeError: (172-184): Mappings cannot be assigned to.

test/libsolidity/syntaxTests/nameAndTypeResolution/025_comparison_of_mapping_types.sol

@@ -1,9 +1,9 @@
 contract C {
     mapping(uint => uint) x;
     function f() public returns (bool ret) {
-        mapping(uint => uint) y = x;
+        mapping(uint => uint) storage y = x;
         return x == y;
     }
 }
 // ----
-// TypeError: (139-145): Operator == not compatible with types mapping(uint256 => uint256) and mapping(uint256 => uint256)
+// TypeError: (147-153): Operator == not compatible with types mapping(uint256 => uint256) and mapping(uint256 => uint256)

test/libsolidity/syntaxTests/nameAndTypeResolution/210_uninitialized_mapping_variable.sol

@@ -1,8 +1,8 @@
 contract C {
     function f() public {
-        mapping(uint => uint) x;
+        mapping(uint => uint) storage x;
         x;
     }
 }
 // ----
-// TypeError: (47-70): Uninitialized mapping. Mappings cannot be created dynamically, you have to assign them from a state variable.
+// TypeError: (47-78): Uninitialized mapping. Mappings cannot be created dynamically, you have to assign them from a state variable.

test/libsolidity/syntaxTests/nameAndTypeResolution/264_mapping_in_memory_array.sol

@@ -1,7 +1,7 @@
 contract C {
     function f(uint size) public {
-        mapping(uint => uint) x = new mapping(uint => uint)[](4);
+        mapping(uint => uint) storage x = new mapping(uint => uint)[](4);
     }
 }
 // ----
-// TypeError: (86-109): Type cannot live outside storage.
+// TypeError: (94-117): Type cannot live outside storage.

test/libsolidity/syntaxTests/nameAndTypeResolution/288_conditional_with_all_types.sol

@@ -71,7 +71,7 @@ contract C {
         (uint n, uint o) = true ? (1, 2) : (3, 4);
         (n, o) = (o, n); // Avoid unused var warning
         // mapping
-        mapping(uint8 => uint8) p = true ? table1 : table2;
+        mapping(uint8 => uint8) storage p = true ? table1 : table2;
         p[0] = 0; // Avoid unused var warning
         // typetype
         uint32 q = true ? uint32(1) : uint32(2);

test/libsolidity/syntaxTests/types/mapping/mapping_data_location_calldata.sol

@@ -0,0 +1,9 @@
+contract c {
+    mapping(uint => uint) y;
+    function f() view public {
+        mapping(uint => uint) calldata x = y;
+        x;
+    }
+}
+// ----
+// TypeError: (81-113): Data location for mappings must be specified as "storage".

test/libsolidity/syntaxTests/types/mapping/mapping_data_location_default.sol

@@ -0,0 +1,9 @@
+contract c {
+    mapping(uint => uint) y;
+    function f() view public {
+        mapping(uint => uint) x = y;
+        x;
+    }
+}
+// ----
+// TypeError: (81-104): Data location for mappings must be specified as "storage".

test/libsolidity/syntaxTests/types/mapping/mapping_data_location_memory.sol

@@ -0,0 +1,9 @@
+contract c {
+    mapping(uint => uint) y;
+    function f() view public {
+        mapping(uint => uint) memory x = y;
+        x;
+    }
+}
+// ----
+// TypeError: (81-111): Data location for mappings must be specified as "storage".