Merge pull request #4415 from ethereum/uninitializedStoragePointer

[BREAKING] Turn uninitialized storage variables into error.

12 files changed, 31 insertions, 84 deletions

diff --git a/Changelog.md b/Changelog.md
index 2ded6bf2..a0471ce5 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -34,6 +34,7 @@ Breaking Changes:
  * Type Checker: Disallow arithmetic operations for boolean variables.
  * Type Checker: Disallow conversions between ``bytesX`` and ``uintY`` of different size.
  * Type Checker: Disallow specifying base constructor arguments multiple times in the same inheritance hierarchy. This was already the case in the experimental 0.5.0 mode.
+ * Type Checker: Disallow uninitialized storage variables. This was already the case in the experimental 0.5.0 mode.
  * Type Checker: Only accept a single ``bytes`` type for ``.call()`` (and family), ``keccak256()``, ``sha256()`` and ``ripemd160()``.
  * Remove obsolete ``std`` directory from the Solidity repository. This means accessing ``https://github.com/ethereum/soldity/blob/develop/std/*.sol`` (or ``https://github.com/ethereum/solidity/std/*.sol`` in Remix) will not be possible.
  * Syntax Checker: Named return values in function types are an error.
diff --git a/docs/frequently-asked-questions.rst b/docs/frequently-asked-questions.rst
index 0d6fa033..75693bdd 100644
--- a/docs/frequently-asked-questions.rst
+++ b/docs/frequently-asked-questions.rst
@@ -306,48 +306,11 @@ independent copy of the state variable is created in memory and
 is another issue). The modifications to this independent copy do not
 carry back to ``data1`` or ``data2``.
 
-A common mistake is to declare a local variable and assume that it will
-be created in memory, although it will be created in storage::
-
-    /// THIS CONTRACT CONTAINS AN ERROR
-
-    pragma solidity ^0.4.0;
-
-    contract C {
-        uint someVariable;
-        uint[] data;
-
-        function f() public {
-            uint[] x;
-            x.push(2);
-            data = x;
-        }
-    }
-
-The type of the local variable ``x`` is ``uint[] storage``, but since
-storage is not dynamically allocated, it has to be assigned from
-a state variable before it can be used. So no space in storage will be
-allocated for ``x``, but instead it functions only as an alias for
-a pre-existing variable in storage.
-
-What will happen is that the compiler interprets ``x`` as a storage
-pointer and will make it point to the storage slot ``0`` by default.
-This has the effect that ``someVariable`` (which resides at storage
-slot ``0``) is modified by ``x.push(2)``.
-
-The correct way to do this is the following::
-
-    pragma solidity ^0.4.0;
-
-    contract C {
-        uint someVariable;
-        uint[] data;
-
-        function f() public {
-            uint[] x = data;
-            x.push(2);
-        }
-    }
+.. warning::
+    Prior to version 0.5.0, a common mistake was to declare a local variable and assume that it will
+    be created in memory, although it will be created in storage. Using such a variable without initializing
+    it could lead to unexpected behavior. Starting from 0.5.0, however, storage variables have to be initialized,
+    which should prevent these kinds of mistakes.
 
 ******************
 Advanced Questions
diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp
index 20e423e6..a11b1879 100644
--- a/libsolidity/analysis/TypeChecker.cpp
+++ b/libsolidity/analysis/TypeChecker.cpp
@@ -1073,10 +1073,7 @@ bool TypeChecker::visit(VariableDeclarationStatement const& _statement)
 				if (varDecl.referenceLocation() == VariableDeclaration::Location::Default)
 					errorText += " Did you mean '<type> memory " + varDecl.name() + "'?";
 				solAssert(m_scope, "");
-				if (v050)
-					m_errorReporter.declarationError(varDecl.location(), errorText);
-				else
-					m_errorReporter.warning(varDecl.location(), errorText);
+				m_errorReporter.declarationError(varDecl.location(), errorText);
 			}
 		}
 		else if (dynamic_cast<MappingType const*>(type(varDecl).get()))
diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp
index ce00931b..fbc6a9c6 100644
--- a/test/libsolidity/SolidityEndToEndTest.cpp
+++ b/test/libsolidity/SolidityEndToEndTest.cpp
@@ -7878,6 +7878,7 @@ BOOST_AUTO_TEST_CASE(tuples)
 	char const* sourceCode = R"(
 		contract C {
 			uint[] data;
+			uint[] m_c;
 			function g() internal returns (uint a, uint b, uint[] storage c) {
 				return (1, 2, data);
 			}
@@ -7890,7 +7891,7 @@ BOOST_AUTO_TEST_CASE(tuples)
 				uint a; uint b;
 				(a, b) = this.h();
 				if (a != 5 || b != 6) return 1;
-				uint[] storage c;
+				uint[] storage c = m_c;
 				(a, b, c) = g();
 				if (a != 1 || b != 2 || c[0] != 3) return 2;
 				(a, b) = (b, a);
@@ -9585,7 +9586,7 @@ BOOST_AUTO_TEST_CASE(calling_uninitialized_function_through_array)
 					{ assembly { mstore(0, 7) return(0, 0x20) } }
 				mutex = 1;
 				// Avoid re-executing this function if we jump somewhere.
-				function() internal returns (uint)[200] x;
+				function() internal returns (uint)[200] memory x;
 				x[0]();
 				return 2;
 			}
@@ -10113,7 +10114,7 @@ BOOST_AUTO_TEST_CASE(copy_internal_function_array_to_storage)
 			function() internal returns (uint)[20] x;
 			int mutex;
 			function one() public returns (uint) {
-				function() internal returns (uint)[20] xmem;
+				function() internal returns (uint)[20] memory xmem;
 				x = xmem;
 				return 3;
 			}
diff --git a/test/libsolidity/syntaxTests/array/uninitialized_storage_var.sol b/test/libsolidity/syntaxTests/array/uninitialized_storage_var.sol
new file mode 100644
index 00000000..363d8147
--- /dev/null
+++ b/test/libsolidity/syntaxTests/array/uninitialized_storage_var.sol
@@ -0,0 +1,9 @@
+contract C {
+	function f() {
+		uint[] storage x;
+		uint[10] storage y;
+	}
+}
+// ----
+// DeclarationError: (31-47): Uninitialized storage pointer.
+// DeclarationError: (51-69): Uninitialized storage pointer.
diff --git a/test/libsolidity/syntaxTests/nameAndTypeResolution/206_storage_location_local_variables.sol b/test/libsolidity/syntaxTests/nameAndTypeResolution/206_storage_location_local_variables.sol
index 5199e5d7..868d7bc8 100644
--- a/test/libsolidity/syntaxTests/nameAndTypeResolution/206_storage_location_local_variables.sol
+++ b/test/libsolidity/syntaxTests/nameAndTypeResolution/206_storage_location_local_variables.sol
@@ -1,11 +1,9 @@
 contract C {
-    function f() public {
-        uint[] storage x;
+    uint[] m_x;
+    function f() public view {
+        uint[] storage x = m_x;
         uint[] memory y;
-        uint[] memory z;
-        x;y;z;
+        x;y;
     }
 }
 // ----
-// Warning: (47-63): Uninitialized storage pointer.
-// Warning: (17-135): Function state mutability can be restricted to pure
diff --git a/test/libsolidity/syntaxTests/nameAndTypeResolution/211_uninitialized_mapping_array_variable.sol b/test/libsolidity/syntaxTests/nameAndTypeResolution/211_uninitialized_mapping_array_variable.sol
index 80467491..edae7549 100644
--- a/test/libsolidity/syntaxTests/nameAndTypeResolution/211_uninitialized_mapping_array_variable.sol
+++ b/test/libsolidity/syntaxTests/nameAndTypeResolution/211_uninitialized_mapping_array_variable.sol
@@ -5,4 +5,4 @@ contract C {
     }
 }
 // ----
-// Warning: (52-85): Uninitialized storage pointer.
+// DeclarationError: (52-85): Uninitialized storage pointer.
diff --git a/test/libsolidity/syntaxTests/nameAndTypeResolution/212_uninitialized_mapping_array_variable_050.sol b/test/libsolidity/syntaxTests/nameAndTypeResolution/212_uninitialized_mapping_array_variable_050.sol
deleted file mode 100644
index f2028690..00000000
--- a/test/libsolidity/syntaxTests/nameAndTypeResolution/212_uninitialized_mapping_array_variable_050.sol
+++ /dev/null
@@ -1,9 +0,0 @@
-pragma experimental "v0.5.0";
-contract C {
-    function f() pure public {
-        mapping(uint => uint)[] storage x;
-        x;
-    }
-}
-// ----
-// DeclarationError: (82-115): Uninitialized storage pointer.
diff --git a/test/libsolidity/syntaxTests/nameAndTypeResolution/233_non_initialized_references.sol b/test/libsolidity/syntaxTests/nameAndTypeResolution/233_non_initialized_references.sol
index 9d8d4834..a0b6f71e 100644
--- a/test/libsolidity/syntaxTests/nameAndTypeResolution/233_non_initialized_references.sol
+++ b/test/libsolidity/syntaxTests/nameAndTypeResolution/233_non_initialized_references.sol
@@ -8,4 +8,4 @@ contract C {
     }
 }
 // ----
-// Warning: (84-95): Uninitialized storage pointer.
+// DeclarationError: (84-95): Uninitialized storage pointer.
diff --git a/test/libsolidity/syntaxTests/nameAndTypeResolution/234_non_initialized_references_050.sol b/test/libsolidity/syntaxTests/nameAndTypeResolution/234_non_initialized_references_050.sol
deleted file mode 100644
index c221b73c..00000000
--- a/test/libsolidity/syntaxTests/nameAndTypeResolution/234_non_initialized_references_050.sol
+++ /dev/null
@@ -1,11 +0,0 @@
-pragma experimental "v0.5.0";
-contract C {
-    struct s {
-        uint a;
-    }
-    function f() public {
-        s storage x;
-    }
-}
-// ----
-// DeclarationError: (114-125): Uninitialized storage pointer.
diff --git a/test/libsolidity/syntaxTests/parsing/arrays_in_expressions.sol b/test/libsolidity/syntaxTests/parsing/arrays_in_expressions.sol
index 95ef66d4..2b35ffda 100644
--- a/test/libsolidity/syntaxTests/parsing/arrays_in_expressions.sol
+++ b/test/libsolidity/syntaxTests/parsing/arrays_in_expressions.sol
@@ -5,4 +5,4 @@ contract c {
 // Warning: (39-46): Variable is declared as a storage pointer. Use an explicit "storage" keyword to silence this warning.
 // Warning: (52-67): Variable is declared as a storage pointer. Use an explicit "storage" keyword to silence this warning.
 // TypeError: (39-50): Type int_const 7 is not implicitly convertible to expected type contract c[10] storage pointer.
-// Warning: (52-67): Uninitialized storage pointer. Did you mean '<type> memory x'?
+// DeclarationError: (52-67): Uninitialized storage pointer. Did you mean '<type> memory x'?
diff --git a/test/libsolidity/syntaxTests/parsing/location_specifiers_for_locals.sol b/test/libsolidity/syntaxTests/parsing/location_specifiers_for_locals.sol
index e311dd96..38de7b1c 100644
--- a/test/libsolidity/syntaxTests/parsing/location_specifiers_for_locals.sol
+++ b/test/libsolidity/syntaxTests/parsing/location_specifiers_for_locals.sol
@@ -1,11 +1,9 @@
 contract Foo {
-    function f() public {
-        uint[] storage x;
+    uint[] m_x;
+    function f() public view {
+        uint[] storage x = m_x;
         uint[] memory y;
+        x; y;
     }
 }
 // ----
-// Warning: (49-65): Uninitialized storage pointer.
-// Warning: (49-65): Unused local variable.
-// Warning: (75-90): Unused local variable.
-// Warning: (19-97): Function state mutability can be restricted to pure