Merge pull request #2528 from ethereum/warnNoStorage

Warn if local storage reference variable does not use "storage" explicitly.

4 files changed, 51 insertions, 7 deletions

diff --git a/Changelog.md b/Changelog.md
index 80e85858..c5085f1b 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -1,5 +1,8 @@
 ### 0.4.13 (unreleased)
 
+Features:
+ * Type Checker: Warn if a local storage reference variable does not explicitly use the keyword ``storage``.
+
 Bugfixes:
  * Compiler Interface: Only output AST if analysis was successful.
  * Code Generator: Correctly unregister modifier variables.
diff --git a/libsolidity/analysis/ReferencesResolver.cpp b/libsolidity/analysis/ReferencesResolver.cpp
index 2a5f27df..cc95c294 100644
--- a/libsolidity/analysis/ReferencesResolver.cpp
+++ b/libsolidity/analysis/ReferencesResolver.cpp
@@ -289,7 +289,20 @@ void ReferencesResolver::endVisit(VariableDeclaration const& _variable)
 					typeLoc = DataLocation::Memory;
 				}
 				else if (varLoc == Location::Default)
-					typeLoc = _variable.isCallableParameter() ? DataLocation::Memory : DataLocation::Storage;
+				{
+					if (_variable.isCallableParameter())
+						typeLoc = DataLocation::Memory;
+					else
+					{
+						typeLoc = DataLocation::Storage;
+						if (!_variable.isStateVariable())
+							m_errorReporter.warning(
+								_variable.location(),
+								"Variable is declared as a storage pointer. "
+								"Use an explicit \"storage\" keyword to silence this warning."
+							);
+					}
+				}
 				else
 					typeLoc = varLoc == Location::Memory ? DataLocation::Memory : DataLocation::Storage;
 				isPointer = !_variable.isStateVariable();
diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp
index 1563467c..7306a36d 100644
--- a/libsolidity/analysis/TypeChecker.cpp
+++ b/libsolidity/analysis/TypeChecker.cpp
@@ -854,10 +854,12 @@ bool TypeChecker::visit(VariableDeclarationStatement const& _statement)
 		if (auto ref = dynamic_cast<ReferenceType const*>(type(varDecl).get()))
 		{
 			if (ref->dataStoredIn(DataLocation::Storage))
-				m_errorReporter.warning(
-					varDecl.location(),
-					"Uninitialized storage pointer. Did you mean '<type> memory " + varDecl.name() + "'?"
-				);
+			{
+				string errorText{"Uninitialized storage pointer."};
+				if (varDecl.referenceLocation() == VariableDeclaration::Location::Default)
+					errorText += " Did you mean '<type> memory " + varDecl.name() + "'?";
+				m_errorReporter.warning(varDecl.location(), errorText);
+			}
 		}
 		else if (dynamic_cast<MappingType const*>(type(varDecl).get()))
 			m_errorReporter.typeError(
diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp
index d0aee3d0..e04d50e8 100644
--- a/test/libsolidity/SolidityNameAndTypeResolution.cpp
+++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp
@@ -2817,7 +2817,7 @@ BOOST_AUTO_TEST_CASE(uninitialized_mapping_array_variable)
 	char const* sourceCode = R"(
 		contract C {
 			function f() {
-				mapping(uint => uint)[] x;
+				mapping(uint => uint)[] storage x;
 				x;
 			}
 		}
@@ -3103,7 +3103,7 @@ BOOST_AUTO_TEST_CASE(non_initialized_references)
 			}
 			function f()
 			{
-				s x;
+				s storage x;
 				x.a = 2;
 			}
 		}
@@ -6144,6 +6144,32 @@ BOOST_AUTO_TEST_CASE(shadowing_warning_can_be_removed)
 	CHECK_SUCCESS_NO_WARNINGS(text);
 }
 
+BOOST_AUTO_TEST_CASE(warn_unspecified_storage)
+{
+	char const* text = R"(
+		contract C {
+			struct S { uint a; }
+			S x;
+			function f() {
+				S storage y = x;
+				y;
+			}
+		}
+	)";
+	CHECK_SUCCESS_NO_WARNINGS(text);
+	text = R"(
+		contract C {
+			struct S { uint a; }
+			S x;
+			function f() {
+				S y = x;
+				y;
+			}
+		}
+	)";
+	CHECK_WARNING(text, "is declared as a storage pointer. Use an explicit \"storage\" keyword to silence this warning");
+}
+
 
 BOOST_AUTO_TEST_SUITE_END()