Merge pull request #12470 from nishant-sachdeva/equality_operator_for_external_functions

Equality operator allowed for external function types

Author: ekpyron

Changelog.md

@@ -2,6 +2,7 @@
 
 Language Features:
  * General: Support ``ContractName.functionName`` for ``abi.encodeCall``, in addition to external function pointers.
+ * General: Add equality-comparison operators for external function types.
 
 
 Compiler Features:

libsolidity/ast/Types.cpp

@@ -3018,8 +3018,18 @@ TypeResult FunctionType::binaryOperatorResult(Token _operator, Type const* _othe
 	if (_other->category() != category() || !(_operator == Token::Equal || _operator == Token::NotEqual))
 		return nullptr;
 	FunctionType const& other = dynamic_cast<FunctionType const&>(*_other);
-	if (kind() == Kind::Internal && other.kind() == Kind::Internal && sizeOnStack() == 1 && other.sizeOnStack() == 1)
+	if (kind() == Kind::Internal && sizeOnStack() == 1 && other.kind() == Kind::Internal && other.sizeOnStack() == 1)
 		return commonType(this, _other);
+	else if (
+		kind() == Kind::External &&
+		sizeOnStack() == 2 &&
+		!bound() &&
+		other.kind() == Kind::External &&
+		other.sizeOnStack() == 2 &&
+		!other.bound()
+	)
+		return commonType(this, _other);
+
 	return nullptr;
 }
 

libsolidity/codegen/ExpressionCompiler.cpp

@@ -2278,26 +2278,44 @@ void ExpressionCompiler::appendAndOrOperatorCode(BinaryOperation const& _binaryO
 
 void ExpressionCompiler::appendCompareOperatorCode(Token _operator, Type const& _type)
 {
-	solAssert(_type.sizeOnStack() == 1, "Comparison of multi-slot types.");
 	if (_operator == Token::Equal || _operator == Token::NotEqual)
 	{
-		if (FunctionType const* funType = dynamic_cast<decltype(funType)>(&_type))
+		FunctionType const* functionType = dynamic_cast<decltype(functionType)>(&_type);
+		if (functionType && functionType->kind() == FunctionType::Kind::External)
 		{
-			if (funType->kind() == FunctionType::Kind::Internal)
+			solUnimplementedAssert(functionType->sizeOnStack() == 2, "");
+			m_context << Instruction::SWAP3;
+
+			m_context << ((u256(1) << 160) - 1) << Instruction::AND;
+			m_context << Instruction::SWAP1;
+			m_context << ((u256(1) << 160) - 1) << Instruction::AND;
+			m_context << Instruction::EQ;
+			m_context << Instruction::SWAP2;
+			m_context << ((u256(1) << 32) - 1) << Instruction::AND;
+			m_context << Instruction::SWAP1;
+			m_context << ((u256(1) << 32) - 1) << Instruction::AND;
+			m_context << Instruction::EQ;
+			m_context << Instruction::AND;
+		}
+		else
+		{
+			solAssert(_type.sizeOnStack() == 1, "Comparison of multi-slot types.");
+			if (functionType && functionType->kind() == FunctionType::Kind::Internal)
 			{
 				// We have to remove the upper bits (construction time value) because they might
 				// be "unknown" in one of the operands and not in the other.
 				m_context << ((u256(1) << 32) - 1) << Instruction::AND;
 				m_context << Instruction::SWAP1;
 				m_context << ((u256(1) << 32) - 1) << Instruction::AND;
 			}
+			m_context << Instruction::EQ;
 		}
-		m_context << Instruction::EQ;
 		if (_operator == Token::NotEqual)
 			m_context << Instruction::ISZERO;
 	}
 	else
 	{
+		solAssert(_type.sizeOnStack() == 1, "Comparison of multi-slot types.");
 		bool isSigned = false;
 		if (auto type = dynamic_cast<IntegerType const*>(&_type))
 			isSigned = type->isSigned();

libsolidity/codegen/YulUtilFunctions.cpp

@@ -29,6 +29,7 @@
 #include <libsolutil/FunctionSelector.h>
 #include <libsolutil/Whiskers.h>
 #include <libsolutil/StringUtils.h>
+#include <libsolidity/ast/TypeProvider.h>
 
 using namespace std;
 using namespace solidity;
@@ -4548,3 +4549,31 @@ string YulUtilFunctions::externalCodeFunction()
 		.render();
 	});
 }
+
+std::string YulUtilFunctions::externalFunctionPointersEqualFunction()
+{
+	std::string const functionName = "externalFunctionPointersEqualFunction";
+	return m_functionCollector.createFunction(functionName, [&]() {
+		return util::Whiskers(R"(
+			function <functionName>(
+				leftAddress,
+				leftSelector,
+				rightAddress,
+				rightSelector
+			) -> result {
+				result := and(
+					eq(
+						<addressCleanUpFunction>(leftAddress), <addressCleanUpFunction>(rightAddress)
+					),
+					eq(
+						<selectorCleanUpFunction>(leftSelector), <selectorCleanUpFunction>(rightSelector)
+					)
+				)
+			}
+		)")
+		("functionName", functionName)
+		("addressCleanUpFunction", cleanupFunction(*TypeProvider::address()))
+		("selectorCleanUpFunction", cleanupFunction(*TypeProvider::uint(32)))
+		.render();
+	});
+}

libsolidity/codegen/YulUtilFunctions.h

@@ -522,6 +522,9 @@ class YulUtilFunctions
 	/// Signature: (address) -> mpos
 	std::string externalCodeFunction();
 
+	/// @return the name of a function that that checks if two external functions pointers are equal or not
+	std::string externalFunctionPointersEqualFunction();
+
 private:
 /// @returns the name of a function that copies a struct from calldata or memory to storage
 	/// signature: (slot, value) ->

libsolidity/codegen/ir/IRGeneratorForStatements.cpp

@@ -799,22 +799,34 @@ bool IRGeneratorForStatements::visit(BinaryOperation const& _binOp)
 
 	if (TokenTraits::isCompareOp(op))
 	{
-		if (auto type = dynamic_cast<FunctionType const*>(commonType))
-		{
-			solAssert(op == Token::Equal || op == Token::NotEqual, "Invalid function pointer comparison!");
-			solAssert(type->kind() != FunctionType::Kind::External, "External function comparison not allowed!");
-		}
-
 		solAssert(commonType->isValueType(), "");
+
 		bool isSigned = false;
 		if (auto type = dynamic_cast<IntegerType const*>(commonType))
 			isSigned = type->isSigned();
 
 		string args = expressionAsType(_binOp.leftExpression(), *commonType, true);
 		args += ", " + expressionAsType(_binOp.rightExpression(), *commonType, true);
 
+		auto functionType = dynamic_cast<FunctionType const*>(commonType);
+		solAssert(functionType ? (op == Token::Equal || op == Token::NotEqual) : true, "Invalid function pointer comparison!");
+
 		string expr;
-		if (op == Token::Equal)
+
+		if (functionType && functionType->kind() ==  FunctionType::Kind::External)
+		{
+			solUnimplementedAssert(functionType->sizeOnStack() == 2, "");
+			expr = m_utils.externalFunctionPointersEqualFunction() +
+				"(" +
+				IRVariable{_binOp.leftExpression()}.part("address").name() + "," +
+				IRVariable{_binOp.leftExpression()}.part("functionSelector").name() + "," +
+				IRVariable{_binOp.rightExpression()}.part("address").name() + "," +
+				IRVariable{_binOp.rightExpression()}.part("functionSelector").name() +
+				")";
+			if (op == Token::NotEqual)
+				expr = "iszero(" + expr + ")";
+		}
+		else if (op == Token::Equal)
 			expr = "eq(" + move(args) + ")";
 		else if (op == Token::NotEqual)
 			expr = "iszero(eq(" + move(args) + "))";

test/libsolidity/semanticTests/functionTypes/comparison_operator_for_external_function_cleans_dirty_bits.sol

@@ -0,0 +1,17 @@
+contract C {
+  function g() external {}
+  function comparison_operators_for_external_function_pointers_with_dirty_bits() external returns (bool) {
+        function() external g_ptr_dirty = this.g;
+        assembly {
+                g_ptr_dirty.address := or(g_ptr_dirty.address, shl(160, sub(0,1)))
+                g_ptr_dirty.selector := or(g_ptr_dirty.selector, shl(32, sub(0,1)))
+        }
+        function() external g_ptr = this.g;
+        return g_ptr == g_ptr_dirty;
+  }
+}
+// ====
+// compileViaYul: also
+// EVMVersion: >=constantinople
+// ----
+// comparison_operators_for_external_function_pointers_with_dirty_bits() -> true

test/libsolidity/semanticTests/functionTypes/comparison_operators_for_external_functions.sol

@@ -0,0 +1,81 @@
+contract C {
+    function f() external {}
+    function g() external {}
+    function h() pure external {}
+    function i() view external {}
+
+    function comparison_operators_for_external_functions() public returns (bool) {
+        assert(
+            this.f != this.g &&
+            this.f != this.h &&
+            this.f != this.i &&
+
+            this.g != this.h &&
+            this.g != this.i &&
+
+            this.h != this.i &&
+
+            this.f == this.f &&
+            this.g == this.g &&
+            this.h == this.h &&
+            this.i == this.i
+        );
+        return true;
+    }
+
+    function comparison_operators_for_local_external_function_pointers() public returns (bool) {
+        function () external f_local = this.f;
+        function () external g_local = this.g;
+        function () external pure h_local = this.h;
+        function () external view i_local = this.i;
+
+        assert(
+            f_local == this.f &&
+            g_local == this.g &&
+            h_local == this.h &&
+            i_local == this.i &&
+
+            f_local != this.g &&
+            f_local != this.h &&
+            f_local != this.i &&
+
+            g_local != this.f &&
+            g_local != this.h &&
+            g_local != this.i &&
+
+            h_local != this.f &&
+            h_local != this.g &&
+            h_local != this.i &&
+
+            i_local != this.f &&
+            i_local != this.g &&
+            i_local != this.h
+        );
+
+        assert(
+            f_local == f_local &&
+            f_local != g_local &&
+            f_local != h_local &&
+            f_local != i_local
+        );
+
+        assert(
+            g_local == g_local &&
+            g_local != h_local &&
+            g_local != i_local
+        );
+
+        assert(
+            h_local == h_local &&
+            i_local == i_local &&
+            h_local != i_local
+        );
+
+        return true;
+    }
+}
+// ====
+// compileViaYul: also
+// ----
+// comparison_operators_for_external_functions() -> true
+// comparison_operators_for_local_external_function_pointers() -> true

test/libsolidity/syntaxTests/functionTypes/comparison_operator_for_external_functions_with_extra_gas_slots.sol

@@ -0,0 +1,12 @@
+contract C {
+    function external_test_function() external {}
+    function comparison_operator_for_external_function_with_extra_slots() external returns (bool) {
+        return (
+            (this.external_test_function{gas: 4} == this.external_test_function) &&
+            (this.external_test_function{gas: 4} == this.external_test_function{gas: 4})
+        );
+    }
+}
+// ----
+// TypeError 2271: (193-259): Operator == not compatible with types function () external and function () external
+// TypeError 2271: (277-351): Operator == not compatible with types function () external and function () external

test/libsolidity/syntaxTests/functionTypes/comparison_operators_between_internal_and_external_function_pointers.sol

@@ -0,0 +1,23 @@
+contract C {
+    function external_test_function() external {}
+    function internal_test_function() internal {}
+
+    function comparison_operator_between_internal_and_external_function_pointers() external returns (bool) {
+        function () external external_function_pointer_local = this.external_test_function;
+        function () internal internal_function_pointer_local = internal_test_function;
+
+        assert(
+            this.external_test_function == external_function_pointer_local &&
+            internal_function_pointer_local == internal_test_function
+        );
+        assert(
+            internal_function_pointer_local != external_function_pointer_local &&
+            internal_test_function != this.external_test_function
+        );
+
+        return true;
+    }
+}
+// ----
+// TypeError 2271: (606-672): Operator != not compatible with types function () and function () external
+// TypeError 2271: (688-741): Operator != not compatible with types function () and function () external