Merge pull request #11475 from ethereum/receive-override

Fix ICE related to receive function having parameters.

Author: hrkrshnn

Changelog.md

@@ -27,6 +27,7 @@ Bugfixes:
  * Type Checker: Fix internal compiler error related to having mapping types in constructor parameter for abstract contracts.
  * Type Checker: Fix internal compiler error when attempting to use an invalid external function type on pre-byzantium EVMs.
  * Type Checker: Make errors about (nested) mapping type in event or error parameter into fatal type errors.
+ * Type Checker: Fix internal compiler error when overriding receive ether function with one having different parameters during inheritance.
 
 
 AST Changes:

libsolidity/analysis/ContractLevelChecker.cpp

@@ -86,6 +86,7 @@ bool ContractLevelChecker::check(ContractDefinition const& _contract)
 
 	checkDuplicateFunctions(_contract);
 	checkDuplicateEvents(_contract);
+	checkReceiveFunction(_contract);
 	m_overrideChecker.check(_contract);
 	checkBaseConstructorArguments(_contract);
 	checkAbstractDefinitions(_contract);
@@ -162,6 +163,35 @@ void ContractLevelChecker::checkDuplicateEvents(ContractDefinition const& _contr
 	findDuplicateDefinitions(events);
 }
 
+void ContractLevelChecker::checkReceiveFunction(ContractDefinition const& _contract)
+{
+	for (FunctionDefinition const* function: _contract.definedFunctions())
+	{
+		solAssert(function, "");
+		if (function->isReceive())
+		{
+			if (function->libraryFunction())
+				m_errorReporter.declarationError(4549_error, function->location(), "Libraries cannot have receive ether functions.");
+
+			if (function->stateMutability() != StateMutability::Payable)
+				m_errorReporter.declarationError(
+					7793_error,
+					function->location(),
+					"Receive ether function must be payable, but is \"" +
+					stateMutabilityToString(function->stateMutability()) +
+					"\"."
+				);
+			if (function->visibility() != Visibility::External)
+				m_errorReporter.declarationError(4095_error, function->location(), "Receive ether function must be defined as \"external\".");
+
+			if (!function->returnParameters().empty())
+				m_errorReporter.fatalDeclarationError(6899_error, function->returnParameterList()->location(), "Receive ether function cannot return values.");
+			if (!function->parameters().empty())
+				m_errorReporter.fatalDeclarationError(6857_error, function->parameterList().location(), "Receive ether function cannot take parameters.");
+		}
+	}
+}
+
 template <class T>
 void ContractLevelChecker::findDuplicateDefinitions(map<string, vector<T>> const& _definitions)
 {

libsolidity/analysis/ContractLevelChecker.h

@@ -63,6 +63,7 @@ class ContractLevelChecker
 	/// arguments and that there is at most one constructor.
 	void checkDuplicateFunctions(ContractDefinition const& _contract);
 	void checkDuplicateEvents(ContractDefinition const& _contract);
+	void checkReceiveFunction(ContractDefinition const& _contract);
 	template <class T>
 	void findDuplicateDefinitions(std::map<std::string, std::vector<T>> const& _definitions);
 	/// Checks for unimplemented functions and modifiers.

libsolidity/analysis/TypeChecker.cpp

@@ -489,8 +489,6 @@ bool TypeChecker::visit(FunctionDefinition const& _function)
 
 	if (_function.isFallback())
 		typeCheckFallbackFunction(_function);
-	else if (_function.isReceive())
-		typeCheckReceiveFunction(_function);
 	else if (_function.isConstructor())
 		typeCheckConstructor(_function);
 
@@ -1881,30 +1879,6 @@ void TypeChecker::typeCheckFallbackFunction(FunctionDefinition const& _function)
 	}
 }
 
-void TypeChecker::typeCheckReceiveFunction(FunctionDefinition const& _function)
-{
-	solAssert(_function.isReceive(), "");
-
-	if (_function.libraryFunction())
-		m_errorReporter.typeError(4549_error, _function.location(), "Libraries cannot have receive ether functions.");
-
-	if (_function.stateMutability() != StateMutability::Payable)
-		m_errorReporter.typeError(
-			7793_error,
-			_function.location(),
-			"Receive ether function must be payable, but is \"" +
-			stateMutabilityToString(_function.stateMutability()) +
-			"\"."
-		);
-	if (_function.visibility() != Visibility::External)
-		m_errorReporter.typeError(4095_error, _function.location(), "Receive ether function must be defined as \"external\".");
-	if (!_function.returnParameters().empty())
-		m_errorReporter.typeError(6899_error, _function.returnParameterList()->location(), "Receive ether function cannot return values.");
-	if (!_function.parameters().empty())
-		m_errorReporter.typeError(6857_error, _function.parameterList().location(), "Receive ether function cannot take parameters.");
-}
-
-
 void TypeChecker::typeCheckConstructor(FunctionDefinition const& _function)
 {
 	solAssert(_function.isConstructor(), "");

libsolidity/analysis/TypeChecker.h

@@ -96,7 +96,6 @@ class TypeChecker: private ASTConstVisitor
 	);
 
 	void typeCheckFallbackFunction(FunctionDefinition const& _function);
-	void typeCheckReceiveFunction(FunctionDefinition const& _function);
 	void typeCheckConstructor(FunctionDefinition const& _function);
 
 	/// Performs general number and type checks of arguments against function call and struct ctor FunctionCall node parameters.

test/libsolidity/syntaxTests/inheritance/fallback_receive/receive_parameter.sol

@@ -0,0 +1,12 @@
+contract C {
+    receive(bytes2) {}
+}
+contract D is C {
+    receive() {}
+}
+// ----
+// SyntaxError 4937: (17-35): No visibility specified. Did you intend to add "external"?
+// SyntaxError 4937: (60-72): No visibility specified. Did you intend to add "external"?
+// DeclarationError 7793: (17-35): Receive ether function must be payable, but is "nonpayable".
+// DeclarationError 4095: (17-35): Receive ether function must be defined as "external".
+// DeclarationError 6857: (24-32): Receive ether function cannot take parameters.

test/libsolidity/syntaxTests/inheritance/fallback_receive/receive_return_parameter.sol

@@ -0,0 +1,8 @@
+contract C {
+    receive() external payable virtual returns(uint) {}
+}
+contract D is C {
+    receive() external payable override {}
+}
+// ----
+// DeclarationError 6899: (59-65): Receive ether function cannot return values.

test/libsolidity/syntaxTests/inheritance/fallback_receive/receive_unimplemented.sol

@@ -0,0 +1,13 @@
+interface I {
+    receive(bytes2) external payable;
+}
+
+interface J is I {
+    receive() external payable override;
+}
+
+contract C is J {
+    receive() external payable override {}
+}
+// ----
+// DeclarationError 6857: (25-33): Receive ether function cannot take parameters.

test/libsolidity/syntaxTests/nameAndTypeResolution/076_receive_function_in_library.sol

@@ -2,5 +2,5 @@ library C {
     receive() external payable {}
 }
 // ----
+// DeclarationError 4549: (16-45): Libraries cannot have receive ether functions.
 // TypeError 7708: (16-45): Library functions cannot be payable.
-// TypeError 4549: (16-45): Libraries cannot have receive ether functions.

test/libsolidity/syntaxTests/receiveEther/arguments.sol

@@ -2,4 +2,4 @@ contract C {
     receive(uint256) external payable {}
 }
 // ----
-// TypeError 6857: (24-33): Receive ether function cannot take parameters.
+// DeclarationError 6857: (24-33): Receive ether function cannot take parameters.