Merge pull request #5487 from ethereum/fixInterfaceOverrideTypeChange

Fix interface override type change

4 files changed, 15 insertions, 4 deletions

diff --git a/Changelog.md b/Changelog.md
index 5408bb8e..b899fa30 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -3,7 +3,6 @@
 Language Features:
  * Allow public functions to override external functions.
 
-
 Compiler Features:
  * Build System: LLL is not built anymore by default. Must configure it with CMake as `-DLLL=ON`.
  * Code generator: Do not perform redundant double cleanup on unsigned integers when loading from calldata.
@@ -16,6 +15,7 @@ Compiler Features:
 Bugfixes:
  * Assembly output: Do not mix in/out jump annotations with arguments.
  * Code Generator: Annotate jump from calldata decoder to function as "jump in".
+ * Type Checker: Properly detect different return types when overriding an external interface function with a public contract function.
 
 Build System:
  * Emscripten: Upgrade to Emscripten SDK 1.37.21 and boost 1.67.
diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp
index 6295e083..1a3844ad 100644
--- a/libsolidity/analysis/TypeChecker.cpp
+++ b/libsolidity/analysis/TypeChecker.cpp
@@ -409,6 +409,8 @@ void TypeChecker::checkFunctionOverride(FunctionDefinition const& _function, Fun
 
 	if (!functionType->hasEqualParameterTypes(*superType))
 		return;
+	if (!functionType->hasEqualReturnTypes(*superType))
+		overrideError(_function, _super, "Overriding function return types differ.");
 
 	if (!_function.annotation().superFunction)
 		_function.annotation().superFunction = &_super;
@@ -423,7 +425,7 @@ void TypeChecker::checkFunctionOverride(FunctionDefinition const& _function, Fun
 		))
 			overrideError(_function, _super, "Overriding function visibility differs.");
 	}
-	else if (_function.stateMutability() != _super.stateMutability())
+	if (_function.stateMutability() != _super.stateMutability())
 		overrideError(
 			_function,
 			_super,
@@ -433,8 +435,6 @@ void TypeChecker::checkFunctionOverride(FunctionDefinition const& _function, Fun
 			stateMutabilityToString(_function.stateMutability()) +
 			"\"."
 		);
-	else if (*functionType != *superType)
-		overrideError(_function, _super, "Overriding function return types differ.");
 }
 
 void TypeChecker::overrideError(FunctionDefinition const& function, FunctionDefinition const& super, string message)
diff --git a/test/libsolidity/syntaxTests/inheritance/override/change_return_types_in_interface.sol b/test/libsolidity/syntaxTests/inheritance/override/change_return_types_in_interface.sol
new file mode 100644
index 00000000..804a1810
--- /dev/null
+++ b/test/libsolidity/syntaxTests/inheritance/override/change_return_types_in_interface.sol
@@ -0,0 +1,10 @@
+interface I {
+  function f() external pure returns (uint);
+}
+contract B is I {
+  // The compiler used to have a bug where changing
+  // the return type was fine in this situation.
+  function f() public pure returns (uint, uint) {}
+}
+// ----
+// TypeError: (182-230): Overriding function return types differ.
diff --git a/test/libsolidity/syntaxTests/nameAndTypeResolution/182_equal_overload.sol b/test/libsolidity/syntaxTests/nameAndTypeResolution/182_equal_overload.sol
index 6f0c7df7..86585518 100644
--- a/test/libsolidity/syntaxTests/nameAndTypeResolution/182_equal_overload.sol
+++ b/test/libsolidity/syntaxTests/nameAndTypeResolution/182_equal_overload.sol
@@ -4,3 +4,4 @@ contract C {
 }
 // ----
 // DeclarationError: (17-66): Function with same name and arguments defined twice.
+// TypeError: (17-66): Overriding function return types differ.