Merge pull request #15174 from ethereum/require-with-error-legacy

Require with custom error for legacy pipeline

Author: nikola-matic

Changelog.md

@@ -2,6 +2,7 @@
 
 Language Features:
  * Accept declarations of state variables with ``transient`` data location (parser support only, no code generation yet).
+ * Make ``require(bool, Error)`` available when using the legacy pipeline.
 
 
 Compiler Features:

docs/contracts/errors.rst

@@ -19,15 +19,14 @@ as well as the newer approach with ``require`` in function ``transferWithRequire
 .. code-block:: solidity
 
     // SPDX-License-Identifier: GPL-3.0
-    pragma solidity ^0.8.26;
+    pragma solidity ^0.8.27;
 
     /// Insufficient balance for transfer. Needed `required` but only
     /// `available` available.
     /// @param available balance available.
     /// @param required requested amount to transfer.
     error InsufficientBalance(uint256 available, uint256 required);
 
-    // This will only compile via IR
     contract TestToken {
         mapping(address => uint) balance;
         function transferWithRevertError(address to, uint256 amount) public {

docs/control-structures.rst

@@ -639,10 +639,6 @@ The ``require`` function provides three overloads:
     For example, in ``require(condition, CustomError(f()));`` and ``require(condition, f());``,
     function ``f()`` will be called regardless of whether the supplied condition is ``true`` or ``false``.
 
-.. note::
-    Using custom errors with ``require`` is only supported by the via IR pipeline, i.e. compilation via Yul.
-    For the legacy pipeline, please use ``if (!condition) revert CustomError();`` instead.
-
 An ``Error(string)`` exception (or an exception without data) is generated
 by the compiler in the following situations:
 

libsolidity/codegen/ExpressionCompiler.cpp

@@ -990,6 +990,8 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall)
 		}
 		case FunctionType::Kind::Error:
 		{
+			// This case is part of the ``revert <error>`` path of the codegen.
+			// For ``require(bool, <error>)`` refer to the ``Kind::Require`` case.
 			_functionCall.expression().accept(*this);
 			std::vector<Type const*> argumentTypes;
 			for (ASTPointer<Expression const> const& arg: _functionCall.sortedArguments())
@@ -1249,21 +1251,58 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall)
 		case FunctionType::Kind::Require:
 		{
 			acceptAndConvert(*arguments.front(), *function.parameterTypes().front(), false);
-
+			// stack: <condition>
 			bool haveReasonString = arguments.size() > 1 && m_context.revertStrings() != RevertStrings::Strip;
-
 			if (arguments.size() > 1)
 			{
 				// Users probably expect the second argument to be evaluated
 				// even if the condition is false, as would be the case for an actual
 				// function call.
 				solAssert(arguments.size() == 2, "");
 				solAssert(function.kind() == FunctionType::Kind::Require, "");
-				// Check if the function call matches ``require(bool, error)``, and throw an unimplemented error,
-				// as require with custom errors is not supported in legacy codegen.
 				auto const* magicType = dynamic_cast<MagicType const*>(arguments[1]->annotation().type);
 				if (magicType && magicType->kind() == MagicType::Kind::Error)
-					solUnimplemented("Require with a custom error is only available using the via-ir pipeline.");
+				{
+					// Make sure that error constructor arguments are evaluated regardless of the require condition
+					auto const& errorConstructorCall = dynamic_cast<FunctionCall const&>(*arguments[1]);
+					errorConstructorCall.expression().accept(*this);
+					std::vector<Type const*> errorConstructorArgumentTypes{};
+					for (ASTPointer<Expression const> const& errorConstructorArgument: errorConstructorCall.sortedArguments())
+					{
+						errorConstructorArgument->accept(*this);
+						errorConstructorArgumentTypes.push_back(errorConstructorArgument->annotation().type);
+					}
+					unsigned const sizeOfConditionArgument = arguments.at(0)->annotation().type->sizeOnStack();
+					unsigned const sizeOfErrorArguments = CompilerUtils::sizeOnStack(errorConstructorArgumentTypes);
+					// stack: <condition> <arg0> <arg1> ... <argN>
+					try
+					{
+						// Move condition to the top of the stack
+						utils().moveToStackTop(sizeOfErrorArguments, sizeOfConditionArgument);
+					}
+					catch (StackTooDeepError const& _exception)
+					{
+						_exception << errinfo_sourceLocation(errorConstructorCall.location());
+						throw _exception;
+					}
+					// stack: <arg0> <arg1> ... <argN> <condition>
+					m_context << Instruction::ISZERO << Instruction::ISZERO;
+					AssemblyItem successBranchTag = m_context.appendConditionalJump();
+
+					auto const* errorDefinition = dynamic_cast<ErrorDefinition const*>(ASTNode::referencedDeclaration(errorConstructorCall.expression()));
+					solAssert(errorDefinition && errorDefinition->functionType(true));
+					utils().revertWithError(
+						errorDefinition->functionType(true)->externalSignature(),
+						errorDefinition->functionType(true)->parameterTypes(),
+						errorConstructorArgumentTypes
+					);
+					// Here, the argument is consumed, but in the other branch, it is still there.
+					m_context.adjustStackOffset(static_cast<int>(sizeOfErrorArguments));
+					m_context << successBranchTag;
+					// In case of the success branch i.e. require(true, ...), pop error constructor arguments
+					utils().popStackSlots(sizeOfErrorArguments);
+					break;
+				}
 
 				if (m_context.revertStrings() == RevertStrings::Strip)
 				{

test/libsolidity/semanticTests/errors/require_different_errors_same_parameters.sol

@@ -14,8 +14,6 @@ contract C
     }
 }
 
-// ====
-// compileViaYul: true
 // ----
 // f() -> FAILURE, hex"f55fefe3", hex"0000000000000000000000000000000000000000000000000000000000000001", hex"0000000000000000000000000000000000000000000000000000000000000060", hex"0000000000000000000000000000000000000000000000000000000000000003", hex"0000000000000000000000000000000000000000000000000000000000000003", hex"74776f0000000000000000000000000000000000000000000000000000000000"
 // g() -> FAILURE, hex"44a06798", hex"0000000000000000000000000000000000000000000000000000000000000004", hex"0000000000000000000000000000000000000000000000000000000000000060", hex"0000000000000000000000000000000000000000000000000000000000000006", hex"0000000000000000000000000000000000000000000000000000000000000004", hex"6669766500000000000000000000000000000000000000000000000000000000"

test/libsolidity/semanticTests/errors/require_error_condition_evaluated_only_once.sol

@@ -0,0 +1,24 @@
+contract C {
+    uint256 counter = 0;
+
+    error CustomError(uint256);
+
+    function getCounter() public view returns (uint256) {
+        return counter;
+    }
+
+    function g(bool condition) internal returns (bool) {
+        counter++;
+        return condition;
+    }
+
+    function f(bool condition) external {
+        require(g(condition), CustomError(counter));
+    }
+}
+
+// ----
+// f(bool): false -> FAILURE, hex"110b3655", 1
+// getCounter() -> 0
+// f(bool): true ->
+// getCounter() -> 1

test/libsolidity/semanticTests/errors/require_evaluation_order.sol renamed to test/libsolidity/semanticTests/errors/require_error_evaluation_order_1.sol

@@ -21,8 +21,6 @@ contract C
     }
 }
 
-// ====
-// compileViaYul: true
 // ----
 // f() -> 7
 // g() -> 7

test/libsolidity/semanticTests/errors/require_error_evaluation_order_2.sol

@@ -0,0 +1,16 @@
+contract C {
+    uint y;
+    function g(bool x) internal returns (bool) {
+        y = 42;
+        return x;
+    }
+    error E(uint256);
+    function h() internal returns (uint256) { return y; }
+    function f(bool c) public {
+        require(g(c), E(h()));
+    }
+}
+
+// ----
+// f(bool): false -> FAILURE, hex"002ff067", 42
+// f(bool): true ->

test/libsolidity/semanticTests/errors/require_error_evaluation_order_3.sol

@@ -0,0 +1,15 @@
+contract C {
+    string failureMessage = "Failure Message";
+    function g(bool x) internal returns (bool) {
+        failureMessage = "Intercepted failure message";
+        return x;
+    }
+    function h() internal returns (string memory) { return failureMessage; }
+    function f(bool c) public {
+        require(g(c), h());
+    }
+}
+
+// ----
+// f(bool): false -> FAILURE, hex"08c379a0", 0x20, 0x1b, "Intercepted failure message"
+// f(bool): true ->

test/libsolidity/semanticTests/errors/require_error_function_join_control_flow.sol

@@ -0,0 +1,18 @@
+contract C {
+    uint x = 0;
+    uint y = 42;
+    error E(uint);
+    function f(bool c) public returns (uint256, uint256, uint256) {
+        uint z = x;
+        if (y == 42) {
+            x = 21;
+        } else {
+            require(c, E(y));
+        }
+        y /= 2;
+        return (x,y,z);
+    }
+}
+// ----
+// f(bool): true -> 0x15, 0x15, 0
+// f(bool): false -> FAILURE, hex"002ff067", 21