Merge pull request #4740 from ethereum/enforceNumberOfReturnValues

Disallow empty return in function with return arguments.

13 files changed, 101 insertions, 38 deletions

diff --git a/Changelog.md b/Changelog.md
index 539d8c61..fc143568 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -46,6 +46,7 @@ Breaking Changes:
  * Type Checker: Disallow calling base constructors without parentheses. This was already the case in the experimental 0.5.0 mode.
  * Type Checker: Disallow conversions between ``bytesX`` and ``uintY`` of different size.
  * Type Checker: Disallow conversions between unrelated contract types. Explicit conversion via ``address`` can still achieve it.
+ * Type Checker: Disallow empty return statements for functions with one or more return values.
  * Type Checker: Disallow empty tuple components. This was partly already the case in the experimental 0.5.0 mode.
  * Type Checker: Disallow multi-variable declarations with mismatching number of values. This was already the case in the experimental 0.5.0 mode.
  * 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.
diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp
index bd3187ae..6882a14f 100644
--- a/libsolidity/analysis/TypeChecker.cpp
+++ b/libsolidity/analysis/TypeChecker.cpp
@@ -982,9 +982,13 @@ bool TypeChecker::visit(ForStatement const& _forStatement)
 
 void TypeChecker::endVisit(Return const& _return)
 {
+	ParameterList const* params = _return.annotation().functionReturnParameters;
 	if (!_return.expression())
+	{
+		if (params && !params->parameters().empty())
+			m_errorReporter.typeError(_return.location(), "Return arguments required.");
 		return;
-	ParameterList const* params = _return.annotation().functionReturnParameters;
+	}
 	if (!params)
 	{
 		m_errorReporter.typeError(_return.location(), "Return arguments not allowed.");
diff --git a/test/compilationTests/milestonetracker/RLP.sol b/test/compilationTests/milestonetracker/RLP.sol
index e1f44457..75e3902e 100644
--- a/test/compilationTests/milestonetracker/RLP.sol
+++ b/test/compilationTests/milestonetracker/RLP.sol
@@ -46,7 +46,6 @@ library RLP {
      subItem = next(self);
      if(strict && !_validate(subItem))
          revert();
-     return;
  }
 
  function hasNext(Iterator memory self) internal view returns (bool) {
@@ -171,10 +170,11 @@ library RLP {
  /// @return The bytes.
  function toBytes(RLPItem memory self) internal returns (bytes memory bts) {
      uint len = self._unsafe_length;
-     if (len == 0)
-         return;
-     bts = new bytes(len);
-     _copyToBytes(self._unsafe_memPtr, bts, len);
+     if (len != 0)
+     {
+         bts = new bytes(len);
+         _copyToBytes(self._unsafe_memPtr, bts, len);
+     }
  }
 
  /// @dev Decode an RLPItem into bytes. This will not work if the
@@ -359,9 +359,8 @@ library RLP {
      if (b0 < DATA_SHORT_START) {
          memPtr = start;
          len = 1;
-         return;
      }
-     if (b0 < DATA_LONG_START) {
+     else if (b0 < DATA_LONG_START) {
          len = self._unsafe_length - 1;
          memPtr = start + 1;
      } else {
@@ -372,7 +371,6 @@ library RLP {
          len = self._unsafe_length - 1 - bLen;
          memPtr = start + bLen + 1;
      }
-     return;
  }
 
  // Assumes that enough memory has been allocated to store in target.
diff --git a/test/contracts/Wallet.cpp b/test/contracts/Wallet.cpp
index dc949063..ce50fe59 100644
--- a/test/contracts/Wallet.cpp
+++ b/test/contracts/Wallet.cpp
@@ -199,7 +199,7 @@ contract multiowned {
 		// determine what index the present sender is:
 		uint ownerIndex = m_ownerIndex[uint(msg.sender)];
 		// make sure they're an owner
-		if (ownerIndex == 0) return;
+		if (ownerIndex == 0) return false;
 
 		PendingState storage pending = m_pending[_operation];
 		// if we're not yet working on this operation, switch over and reset the confirmation status.
@@ -228,6 +228,7 @@ contract multiowned {
 				// not enough: record that this owner in particular confirmed.
 				pending.yetNeeded--;
 				pending.ownersDone |= ownerIndexBit;
+				return false;
 			}
 		}
 	}
diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/emptyReturn_err.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/emptyReturn_err.sol
deleted file mode 100644
index 0d266ccf..00000000
--- a/test/libsolidity/syntaxTests/controlFlow/storageReturn/emptyReturn_err.sol
+++ /dev/null
@@ -1,14 +0,0 @@
-contract C {
-    struct S { bool f; }
-    S s;
-    function f() internal pure returns (S storage) { return; }
-    function g() internal view returns (S storage c, S storage) { c = s; return; }
-    function h() internal view returns (S storage, S storage d) { d = s; return; }
-    function i() internal pure returns (S storage, S storage) { return; }
-}
-// ----
-// TypeError: (87-88): This variable is of storage pointer type and might be returned without assignment and could be used uninitialized. Assign the variable (potentially from itself) to fix this error.
-// TypeError: (163-164): This variable is of storage pointer type and might be returned without assignment and could be used uninitialized. Assign the variable (potentially from itself) to fix this error.
-// TypeError: (233-234): This variable is of storage pointer type and might be returned without assignment and could be used uninitialized. Assign the variable (potentially from itself) to fix this error.
-// TypeError: (316-317): This variable is of storage pointer type and might be returned without assignment and could be used uninitialized. Assign the variable (potentially from itself) to fix this error.
-// TypeError: (327-328): This variable is of storage pointer type and might be returned without assignment and could be used uninitialized. Assign the variable (potentially from itself) to fix this error.
diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/emptyReturn_fine.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/emptyReturn_fine.sol
deleted file mode 100644
index 6d72e4ef..00000000
--- a/test/libsolidity/syntaxTests/controlFlow/storageReturn/emptyReturn_fine.sol
+++ /dev/null
@@ -1,7 +0,0 @@
-contract C {
-    struct S { bool f; }
-    S s;
-    function f() internal view returns (S storage c, S storage d) { c = s; d = s; return; }
-    function g() internal view returns (S storage, S storage) { return (s,s); }
-}
-// ----
diff --git a/test/libsolidity/syntaxTests/controlFlow/storageReturn/tuple_fine.sol b/test/libsolidity/syntaxTests/controlFlow/storageReturn/tuple_fine.sol
index 0b171560..7567f694 100644
--- a/test/libsolidity/syntaxTests/controlFlow/storageReturn/tuple_fine.sol
+++ b/test/libsolidity/syntaxTests/controlFlow/storageReturn/tuple_fine.sol
@@ -8,5 +8,8 @@ contract C {
         uint a;
         (c, a) = f();
     }
+    function h() internal view returns (S storage, S storage) {
+        return (s,s);
+    }
 }
 // ----
diff --git a/test/libsolidity/syntaxTests/nameAndTypeResolution/440_warn_unused_return_parameter_with_explicit_return.sol b/test/libsolidity/syntaxTests/nameAndTypeResolution/440_warn_unused_return_parameter_with_explicit_return.sol
deleted file mode 100644
index af67f491..00000000
--- a/test/libsolidity/syntaxTests/nameAndTypeResolution/440_warn_unused_return_parameter_with_explicit_return.sol
+++ /dev/null
@@ -1,7 +0,0 @@
-contract C {
-    function f() pure public returns (uint a) {
-        return;
-    }
-}
-// ----
-// Warning: (51-57): Unused function parameter. Remove or comment out the variable name to silence this warning.
diff --git a/test/libsolidity/syntaxTests/returnExpressions/single_return_mismatching_number.sol b/test/libsolidity/syntaxTests/returnExpressions/single_return_mismatching_number.sol
new file mode 100644
index 00000000..9741fdfb
--- /dev/null
+++ b/test/libsolidity/syntaxTests/returnExpressions/single_return_mismatching_number.sol
@@ -0,0 +1,14 @@
+contract C
+{
+    function f() public pure returns (uint)
+    {
+        return;
+    }
+    function g() public pure returns (uint)
+    {
+        return (1, 2);
+    }
+}
+// ----
+// TypeError: (71-78): Return arguments required.
+// TypeError: (143-156): Different number of arguments in return statement than in returns declaration.
diff --git a/test/libsolidity/syntaxTests/returnExpressions/single_return_mismatching_number_named.sol b/test/libsolidity/syntaxTests/returnExpressions/single_return_mismatching_number_named.sol
new file mode 100644
index 00000000..53f2d994
--- /dev/null
+++ b/test/libsolidity/syntaxTests/returnExpressions/single_return_mismatching_number_named.sol
@@ -0,0 +1,14 @@
+contract C
+{
+    function f() public pure returns (uint a)
+    {
+        return;
+    }
+    function g() public pure returns (uint a)
+    {
+        return (1, 2);
+    }
+}
+// ----
+// TypeError: (73-80): Return arguments required.
+// TypeError: (147-160): Different number of arguments in return statement than in returns declaration.
diff --git a/test/libsolidity/syntaxTests/returnExpressions/tuple_return_mismatching_number.sol b/test/libsolidity/syntaxTests/returnExpressions/tuple_return_mismatching_number.sol
new file mode 100644
index 00000000..4ea61c68
--- /dev/null
+++ b/test/libsolidity/syntaxTests/returnExpressions/tuple_return_mismatching_number.sol
@@ -0,0 +1,19 @@
+contract C
+{
+    function f() public pure returns (uint, uint)
+    {
+        return 1;
+    }
+    function g() public pure returns (uint, uint)
+    {
+        return (1, 2, 3);
+    }
+    function h() public pure returns (uint, uint)
+    {
+        return;
+    }
+}
+// ----
+// TypeError: (77-85): Different number of arguments in return statement than in returns declaration.
+// TypeError: (157-173): Different number of arguments in return statement than in returns declaration.
+// TypeError: (245-252): Return arguments required.
diff --git a/test/libsolidity/syntaxTests/returnExpressions/tuple_return_mismatching_number_named.sol b/test/libsolidity/syntaxTests/returnExpressions/tuple_return_mismatching_number_named.sol
new file mode 100644
index 00000000..86049719
--- /dev/null
+++ b/test/libsolidity/syntaxTests/returnExpressions/tuple_return_mismatching_number_named.sol
@@ -0,0 +1,19 @@
+contract C
+{
+    function f() public pure returns (uint a, uint b)
+    {
+        return 1;
+    }
+    function g() public pure returns (uint a, uint b)
+    {
+        return (1, 2, 3);
+    }
+    function h() public pure returns (uint a, uint b)
+    {
+        return;
+    }
+}
+// ----
+// TypeError: (81-89): Different number of arguments in return statement than in returns declaration.
+// TypeError: (165-181): Different number of arguments in return statement than in returns declaration.
+// TypeError: (257-264): Return arguments required.
diff --git a/test/libsolidity/syntaxTests/returnExpressions/valid_returns.sol b/test/libsolidity/syntaxTests/returnExpressions/valid_returns.sol
new file mode 100644
index 00000000..e30f9173
--- /dev/null
+++ b/test/libsolidity/syntaxTests/returnExpressions/valid_returns.sol
@@ -0,0 +1,18 @@
+contract C
+{
+    function f() public pure {
+        return;
+    }
+    function g() public pure returns (uint) {
+        return 1;
+    }
+    function h() public pure returns (uint a) {
+        return 1;
+    }
+    function i() public pure returns (uint, uint) {
+        return (1, 2);
+    }
+    function j() public pure returns (uint a, uint b) {
+        return (1, 2);
+    }
+}