Merge pull request #5445 from ethereum/publicExternalOverwrite

Allow overwriting external functions (with ``calldata`` arguments) with public functions (with ``memory`` arguments)

10 files changed, 176 insertions, 8 deletions

diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp
index d503b9ec..6295e083 100644
--- a/libsolidity/analysis/TypeChecker.cpp
+++ b/libsolidity/analysis/TypeChecker.cpp
@@ -215,7 +215,9 @@ void TypeChecker::findDuplicateDefinitions(map<string, vector<T>> const& _defini
 			SecondarySourceLocation ssl;
 
 			for (size_t j = i + 1; j < overloads.size(); ++j)
-				if (FunctionType(*overloads[i]).hasEqualParameterTypes(FunctionType(*overloads[j])))
+				if (FunctionType(*overloads[i]).asCallableFunction(false)->hasEqualParameterTypes(
+					*FunctionType(*overloads[j]).asCallableFunction(false))
+				)
 				{
 					ssl.append("Other declaration is here:", overloads[j]->location());
 					reported.insert(j);
@@ -250,7 +252,7 @@ void TypeChecker::checkContractAbstractFunctions(ContractDefinition const& _cont
 			if (function->isConstructor())
 				continue;
 			auto& overloads = functions[function->name()];
-			FunctionTypePointer funType = make_shared<FunctionType>(*function);
+			FunctionTypePointer funType = make_shared<FunctionType>(*function)->asCallableFunction(false);
 			auto it = find_if(overloads.begin(), overloads.end(), [&](FunTypeAndFlag const& _funAndFlag)
 			{
 				return funType->hasEqualParameterTypes(*_funAndFlag.first);
@@ -402,10 +404,10 @@ void TypeChecker::checkContractIllegalOverrides(ContractDefinition const& _contr
 
 void TypeChecker::checkFunctionOverride(FunctionDefinition const& _function, FunctionDefinition const& _super)
 {
-	FunctionType functionType(_function);
-	FunctionType superType(_super);
+	FunctionTypePointer functionType = FunctionType(_function).asCallableFunction(false);
+	FunctionTypePointer superType = FunctionType(_super).asCallableFunction(false);
 
-	if (!functionType.hasEqualParameterTypes(superType))
+	if (!functionType->hasEqualParameterTypes(*superType))
 		return;
 
 	if (!_function.annotation().superFunction)
@@ -431,7 +433,7 @@ void TypeChecker::checkFunctionOverride(FunctionDefinition const& _function, Fun
 			stateMutabilityToString(_function.stateMutability()) +
 			"\"."
 		);
-	else if (functionType != superType)
+	else if (*functionType != *superType)
 		overrideError(_function, _super, "Overriding function return types differ.");
 }
 
@@ -456,7 +458,7 @@ void TypeChecker::checkContractExternalTypeClashes(ContractDefinition const& _co
 				// under non error circumstances this should be true
 				if (functionType->interfaceFunctionType())
 					externalDeclarations[functionType->externalSignature()].push_back(
-						make_pair(f, functionType)
+						make_pair(f, functionType->asCallableFunction(false))
 					);
 			}
 		for (VariableDeclaration const* v: contract->stateVariables())
@@ -466,7 +468,7 @@ void TypeChecker::checkContractExternalTypeClashes(ContractDefinition const& _co
 				// under non error circumstances this should be true
 				if (functionType->interfaceFunctionType())
 					externalDeclarations[functionType->externalSignature()].push_back(
-						make_pair(v, functionType)
+						make_pair(v, functionType->asCallableFunction(false))
 					);
 			}
 	}
diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp
index e9667483..c34a1399 100644
--- a/test/libsolidity/SolidityEndToEndTest.cpp
+++ b/test/libsolidity/SolidityEndToEndTest.cpp
@@ -8207,6 +8207,58 @@ BOOST_AUTO_TEST_CASE(inherited_function) {
 	ABI_CHECK(callContractFunction("g()"), encodeArgs(u256(1)));
 }
 
+BOOST_AUTO_TEST_CASE(inherited_function_calldata_memory) {
+	char const* sourceCode = R"(
+		contract A { function f(uint[] calldata a) external returns (uint) { return a[0]; } }
+		contract B is A {
+			function f(uint[] memory a) public returns (uint) { return a[1]; }
+			function g() public returns (uint) {
+				uint[] memory m = new uint[](2);
+				m[0] = 42;
+				m[1] = 23;
+				return A(this).f(m);
+			}
+		}
+	)";
+
+	compileAndRun(sourceCode, 0, "B");
+	ABI_CHECK(callContractFunction("g()"), encodeArgs(u256(23)));
+}
+
+BOOST_AUTO_TEST_CASE(inherited_function_calldata_memory_interface) {
+	char const* sourceCode = R"(
+		interface I { function f(uint[] calldata a) external returns (uint); }
+		contract A is I { function f(uint[] memory a) public returns (uint) { return 42; } }
+		contract B {
+			function f(uint[] memory a) public returns (uint) { return a[1]; }
+			function g() public returns (uint) {
+				I i = I(new A());
+				return i.f(new uint[](2));
+			}
+		}
+	)";
+
+	compileAndRun(sourceCode, 0, "B");
+	ABI_CHECK(callContractFunction("g()"), encodeArgs(u256(42)));
+}
+
+BOOST_AUTO_TEST_CASE(inherited_function_calldata_calldata_interface) {
+	char const* sourceCode = R"(
+		interface I { function f(uint[] calldata a) external returns (uint); }
+		contract A is I { function f(uint[] calldata a) external returns (uint) { return 42; } }
+		contract B {
+			function f(uint[] memory a) public returns (uint) { return a[1]; }
+			function g() public returns (uint) {
+				I i = I(new A());
+				return i.f(new uint[](2));
+			}
+		}
+	)";
+
+	compileAndRun(sourceCode, 0, "B");
+	ABI_CHECK(callContractFunction("g()"), encodeArgs(u256(42)));
+}
+
 BOOST_AUTO_TEST_CASE(inherited_function_from_a_library) {
 	char const* sourceCode = R"(
 		library A { function f() internal returns (uint) { return 1; } }
diff --git a/test/libsolidity/syntaxTests/inheritance/override/calldata_memory.sol b/test/libsolidity/syntaxTests/inheritance/override/calldata_memory.sol
new file mode 100644
index 00000000..e683ef39
--- /dev/null
+++ b/test/libsolidity/syntaxTests/inheritance/override/calldata_memory.sol
@@ -0,0 +1,13 @@
+contract A {
+    uint dummy;
+    function f(uint[] calldata) external pure {}
+    function g(uint[] calldata) external view { dummy; }
+    function h(uint[] calldata) external { dummy = 42; }
+    function i(uint[] calldata) external payable {}
+}
+contract B is A {
+    function f(uint[] memory) public pure {}
+    function g(uint[] memory) public view { dummy; }
+    function h(uint[] memory) public { dummy = 42; }
+    function i(uint[] memory) public payable {}
+}
diff --git a/test/libsolidity/syntaxTests/inheritance/override/calldata_memory_conflict.sol b/test/libsolidity/syntaxTests/inheritance/override/calldata_memory_conflict.sol
new file mode 100644
index 00000000..dc734d36
--- /dev/null
+++ b/test/libsolidity/syntaxTests/inheritance/override/calldata_memory_conflict.sol
@@ -0,0 +1,26 @@
+contract A {
+    uint dummy;
+    function f(uint[] calldata) external pure {}
+    function g(uint[] calldata) external view { dummy; }
+    function h(uint[] calldata) external { dummy = 42; }
+    function i(uint[] calldata) external payable {}
+}
+contract B is A {
+    function f(uint[] calldata) external pure {}
+    function g(uint[] calldata) external view { dummy; }
+    function h(uint[] calldata) external { dummy = 42; }
+    function i(uint[] calldata) external payable {}
+    function f(uint[] memory) public pure {}
+    function g(uint[] memory) public view { dummy; }
+    function h(uint[] memory) public { dummy = 42; }
+    function i(uint[] memory) public payable {}
+}
+// ----
+// DeclarationError: (268-312): Function with same name and arguments defined twice.
+// DeclarationError: (317-369): Function with same name and arguments defined twice.
+// DeclarationError: (374-426): Function with same name and arguments defined twice.
+// DeclarationError: (431-478): Function with same name and arguments defined twice.
+// TypeError: (268-312): Overriding function visibility differs.
+// TypeError: (317-369): Overriding function visibility differs.
+// TypeError: (374-426): Overriding function visibility differs.
+// TypeError: (431-478): Overriding function visibility differs.
diff --git a/test/libsolidity/syntaxTests/inheritance/override/calldata_memory_interface.sol b/test/libsolidity/syntaxTests/inheritance/override/calldata_memory_interface.sol
new file mode 100644
index 00000000..7eecc079
--- /dev/null
+++ b/test/libsolidity/syntaxTests/inheritance/override/calldata_memory_interface.sol
@@ -0,0 +1,13 @@
+interface I {
+    function f(uint[] calldata) external pure;
+    function g(uint[] calldata) external view;
+    function h(uint[] calldata) external;
+    function i(uint[] calldata) external payable;
+}
+contract C is I {
+    uint dummy;
+    function f(uint[] memory) public pure {}
+    function g(uint[] memory) public view { dummy; }
+    function h(uint[] memory) public { dummy = 42; }
+    function i(uint[] memory) public payable {}
+}
diff --git a/test/libsolidity/syntaxTests/inheritance/override/calldata_memory_interface_instantiate.sol b/test/libsolidity/syntaxTests/inheritance/override/calldata_memory_interface_instantiate.sol
new file mode 100644
index 00000000..4cdc3924
--- /dev/null
+++ b/test/libsolidity/syntaxTests/inheritance/override/calldata_memory_interface_instantiate.sol
@@ -0,0 +1,12 @@
+interface I {
+    function f(uint[] calldata) external pure;
+}
+contract A is I {
+    function f(uint[] memory) public pure {}
+}
+contract C {
+    function f() public {
+        I i = I(new A());
+        i.f(new uint[](1));
+    }
+}
\ No newline at end of file
diff --git a/test/libsolidity/syntaxTests/inheritance/override/calldata_memory_interface_struct.sol b/test/libsolidity/syntaxTests/inheritance/override/calldata_memory_interface_struct.sol
new file mode 100644
index 00000000..49b27fd7
--- /dev/null
+++ b/test/libsolidity/syntaxTests/inheritance/override/calldata_memory_interface_struct.sol
@@ -0,0 +1,17 @@
+pragma experimental ABIEncoderV2;
+interface I {
+    struct S { int a; }
+    function f(S calldata) external pure;
+    function g(S calldata) external view;
+    function h(S calldata) external;
+    function i(S calldata) external payable;
+}
+contract C is I {
+    uint dummy;
+    function f(S memory) public pure {}
+    function g(S memory) public view { dummy; }
+    function h(S memory) public { dummy = 42; }
+    function i(S memory) public payable {}
+}
+// ----
+// Warning: (0-33): Experimental features are turned on. Do not use experimental features on live deployments.
diff --git a/test/libsolidity/syntaxTests/inheritance/override/calldata_memory_struct.sol b/test/libsolidity/syntaxTests/inheritance/override/calldata_memory_struct.sol
new file mode 100644
index 00000000..42aebf30
--- /dev/null
+++ b/test/libsolidity/syntaxTests/inheritance/override/calldata_memory_struct.sol
@@ -0,0 +1,17 @@
+pragma experimental ABIEncoderV2;
+contract A {
+    uint dummy;
+    struct S { int a; }
+    function f(S calldata) external pure {}
+    function g(S calldata) external view { dummy; }
+    function h(S calldata) external { dummy = 42; }
+    function i(S calldata) external payable {}
+}
+contract B is A {
+    function f(S memory) public pure {}
+    function g(S memory) public view { dummy; }
+    function h(S memory) public { dummy = 42; }
+    function i(S memory) public payable {}
+}
+// ----
+// Warning: (0-33): Experimental features are turned on. Do not use experimental features on live deployments.
diff --git a/test/libsolidity/syntaxTests/inheritance/override/internal_external.sol b/test/libsolidity/syntaxTests/inheritance/override/internal_external.sol
new file mode 100644
index 00000000..90973ee7
--- /dev/null
+++ b/test/libsolidity/syntaxTests/inheritance/override/internal_external.sol
@@ -0,0 +1,7 @@
+contract A {
+    function f(uint[] calldata) external pure {}
+    function f(uint[] memory) internal pure {}
+}
+// ----
+// DeclarationError: (17-61): Function with same name and arguments defined twice.
+// TypeError: (17-61): Overriding function visibility differs.
diff --git a/test/libsolidity/syntaxTests/inheritance/override/internal_external_inheritance.sol b/test/libsolidity/syntaxTests/inheritance/override/internal_external_inheritance.sol
new file mode 100644
index 00000000..c09a8000
--- /dev/null
+++ b/test/libsolidity/syntaxTests/inheritance/override/internal_external_inheritance.sol
@@ -0,0 +1,9 @@
+contract A {
+    function f(uint[] calldata) external pure {}
+}
+contract B {
+    function f(uint[] memory) internal pure {}
+}
+contract C is A, B {}
+// ----
+// TypeError: (81-123): Overriding function visibility differs.