Merge pull request #12628 from nishant-sachdeva/adding_functiontype_string_and_allow_string_concat_operations

Added support for FunctionType::Kind::StringConcat

Author: ekpyron

Changelog.md

@@ -17,6 +17,7 @@ Bugfixes:
  * Code Generator: Fix ICE when accessing the members of external functions occupying more than two stack slots.
  * Code Generator: Fix ICE when doing an explicit conversion from ``string calldata`` to ``bytes``.
  * Control Flow Graph: Perform proper virtual lookup for modifiers for uninitialized variable and unreachable code analysis.
+ * General: ``string.concat`` now properly takes strings as arguments and returns ``string memory``. It was accidentally introduced as a copy of ``bytes.concat`` before.
  * Immutables: Fix wrong error when the constructor of a base contract uses ``return`` and the derived contract contains immutable variables.
  * IR Generator: Add missing cleanup during the conversion of fixed bytes types to smaller fixed bytes types.
  * IR Generator: Add missing cleanup for indexed event arguments of value type.

docs/cheatsheet.rst

@@ -86,6 +86,8 @@ Global Variables
   to ``abi.encodeWithSelector(bytes4(keccak256(bytes(signature)), ...)``
 - ``bytes.concat(...) returns (bytes memory)``: :ref:`Concatenates variable number of
   arguments to one byte array<bytes-concat>`
+- ``string.concat(...) returns (string memory)``: :ref:`Concatenates variable number of
+  arguments to one string array<string-concat>`
 - ``block.basefee`` (``uint``): current block's base fee (`EIP-3198 <https://eips.ethereum.org/EIPS/eip-3198>`_ and `EIP-1559 <https://eips.ethereum.org/EIPS/eip-1559>`_)
 - ``block.chainid`` (``uint``): current chain id
 - ``block.coinbase`` (``address payable``): current block miner's address

docs/types/reference-types.rst

@@ -150,7 +150,7 @@ length or index access.
 Solidity does not have string manipulation functions, but there are
 third-party string libraries. You can also compare two strings by their keccak256-hash using
 ``keccak256(abi.encodePacked(s1)) == keccak256(abi.encodePacked(s2))`` and
-concatenate two strings using ``bytes.concat(bytes(s1), bytes(s2))``.
+concatenate two strings using ``string.concat(s1, s2)``.
 
 You should use ``bytes`` over ``bytes1[]`` because it is cheaper,
 since using ``bytes1[]`` in ``memory`` adds 31 padding bytes between the elements. Note that in ``storage``, the
@@ -165,31 +165,40 @@ always use one of the value types ``bytes1`` to ``bytes32`` because they are muc
     that you are accessing the low-level bytes of the UTF-8 representation,
     and not the individual characters.
 
-.. index:: ! bytes-concat
+.. index:: ! bytes-concat, ! string-concat
 
 .. _bytes-concat:
+.. _string-concat:
 
-``bytes.concat`` function
-^^^^^^^^^^^^^^^^^^^^^^^^^
+The functions ``bytes.concat`` and ``string.concat``
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
-You can concatenate a variable number of ``bytes`` or ``bytes1 ... bytes32`` using ``bytes.concat``.
+You can concatenate an arbitrary number of ``string`` values using ``string.concat``.
+The function returns a single ``string memory`` array that contains the contents of the arguments without padding.
+If you want to use parameters of other types that are not implicitly convertible to ``string``, you need to convert them to ``string`` first.
+
+Analogously, the ``bytes.concat`` function can concatenate an arbitrary number of ``bytes`` or ``bytes1 ... bytes32`` values.
 The function returns a single ``bytes memory`` array that contains the contents of the arguments without padding.
-If you want to use string parameters or other types, you need to convert them to ``bytes`` or ``bytes1``/.../``bytes32`` first.
+If you want to use string parameters or other types that are not implicitly convertible to ``bytes``, you need to convert them to ``bytes`` or ``bytes1``/.../``bytes32`` first.
+
 
 .. code-block:: solidity
 
     // SPDX-License-Identifier: GPL-3.0
-    pragma solidity ^0.8.4;
+    pragma solidity ^0.8.12;
 
     contract C {
-        bytes s = "Storage";
-        function f(bytes calldata c, string memory m, bytes16 b) public view {
-            bytes memory a = bytes.concat(s, c, c[:2], "Literal", bytes(m), b);
-            assert((s.length + c.length + 2 + 7 + bytes(m).length + 16) == a.length);
+        string s = "Storage";
+        function f(bytes calldata bc, string memory sm, bytes16 b) public view {
+            string memory concat_string = string.concat(s, string(bc), "Literal", sm);
+            assert((bytes(s).length + bc.length + 7 + bytes(sm).length) == bytes(concat_string).length);
+
+            bytes memory concat_bytes = bytes.concat(bytes(s), bc, bc[:2], "Literal", bytes(sm), b);
+            assert((bytes(s).length + bc.length + 2 + 7 + bytes(sm).length + b.length) == concat_bytes.length);
         }
     }
 
-If you call ``bytes.concat`` without arguments it will return an empty ``bytes`` array.
+If you call ``string.concat`` or ``bytes.concat`` without arguments they return an empty array.
 
 .. index:: ! array;allocating, new
 

docs/units-and-global-variables.rst

@@ -154,6 +154,14 @@ Members of bytes
 
 - ``bytes.concat(...) returns (bytes memory)``: :ref:`Concatenates variable number of bytes and bytes1, ..., bytes32 arguments to one byte array<bytes-concat>`
 
+.. index:: string members
+
+Members of string
+-----------------
+
+- ``string.concat(...) returns (string memory)``: :ref:`Concatenates variable number of string arguments to one string array<string-concat>`
+
+
 .. index:: assert, revert, require
 
 Error Handling

libsolidity/analysis/TypeChecker.cpp

@@ -2207,14 +2207,42 @@ void TypeChecker::typeCheckABIEncodeCallFunction(FunctionCall const& _functionCa
 	}
 }
 
+
+void TypeChecker::typeCheckStringConcatFunction(
+	FunctionCall const& _functionCall,
+	FunctionType const* _functionType
+)
+{
+	solAssert(_functionType);
+	solAssert(_functionType->kind() == FunctionType::Kind::StringConcat);
+	solAssert(_functionCall.names().empty());
+
+	typeCheckFunctionGeneralChecks(_functionCall, _functionType);
+
+	for (shared_ptr<Expression const> const& argument: _functionCall.arguments())
+	{
+		Type const* argumentType = type(*argument);
+		bool notConvertibleToString = !argumentType->isImplicitlyConvertibleTo(*TypeProvider::stringMemory());
+
+		if (notConvertibleToString)
+			m_errorReporter.typeError(
+				9977_error,
+				argument->location(),
+				"Invalid type for argument in the string.concat function call. "
+				"string type is required, but " +
+				argumentType->identifier() + " provided."
+			);
+	}
+}
+
 void TypeChecker::typeCheckBytesConcatFunction(
 	FunctionCall const& _functionCall,
 	FunctionType const* _functionType
 )
 {
-	solAssert(_functionType, "");
-	solAssert(_functionType->kind() == FunctionType::Kind::BytesConcat, "");
-	solAssert(_functionCall.names().empty(), "");
+	solAssert(_functionType);
+	solAssert(_functionType->kind() == FunctionType::Kind::BytesConcat);
+	solAssert(_functionCall.names().empty());
 
 	typeCheckFunctionGeneralChecks(_functionCall, _functionType);
 
@@ -2651,6 +2679,12 @@ bool TypeChecker::visit(FunctionCall const& _functionCall)
 			returnTypes = functionType->returnParameterTypes();
 			break;
 		}
+		case FunctionType::Kind::StringConcat:
+		{
+			typeCheckStringConcatFunction(_functionCall, functionType);
+			returnTypes = functionType->returnParameterTypes();
+			break;
+		}
 		case FunctionType::Kind::Wrap:
 		case FunctionType::Kind::Unwrap:
 		{

libsolidity/analysis/TypeChecker.h

@@ -113,6 +113,12 @@ class TypeChecker: private ASTConstVisitor
 	/// Performs checks specific to the ABI encode functions of type ABIEncodeCall
 	void typeCheckABIEncodeCallFunction(FunctionCall const& _functionCall);
 
+	/// Performs general checks and checks specific to string concat function call
+	void typeCheckStringConcatFunction(
+		FunctionCall const& _functionCall,
+		FunctionType const* _functionType
+	);
+
 	/// Performs general checks and checks specific to bytes concat function call
 	void typeCheckBytesConcatFunction(
 		FunctionCall const& _functionCall,

libsolidity/ast/Types.cpp

@@ -2928,6 +2928,7 @@ string FunctionType::richIdentifier() const
 	case Kind::ArrayPush: id += "arraypush"; break;
 	case Kind::ArrayPop: id += "arraypop"; break;
 	case Kind::BytesConcat: id += "bytesconcat"; break;
+	case Kind::StringConcat: id += "stringconcat"; break;
 	case Kind::ObjectCreation: id += "objectcreation"; break;
 	case Kind::Assert: id += "assert"; break;
 	case Kind::Require: id += "require"; break;
@@ -3817,15 +3818,14 @@ MemberList::MemberMap TypeType::nativeMembers(ASTNode const* _currentScope) cons
 	)
 		members.emplace_back("concat", TypeProvider::function(
 			TypePointers{},
-			TypePointers{TypeProvider::bytesMemory()},
+			TypePointers{arrayType->isString() ? TypeProvider::stringMemory() : TypeProvider::bytesMemory()},
 			strings{},
-			strings{string()},
-			FunctionType::Kind::BytesConcat,
+			strings{string{}},
+			arrayType->isString() ? FunctionType::Kind::StringConcat : FunctionType::Kind::BytesConcat,
 			StateMutability::Pure,
 			nullptr,
 			FunctionType::Options::withArbitraryParameters()
 		));
-
 	return members;
 }
 

libsolidity/ast/Types.h

@@ -1228,6 +1228,7 @@ class FunctionType: public Type
 		ArrayPush, ///< .push() to a dynamically sized array in storage
 		ArrayPop, ///< .pop() from a dynamically sized array in storage
 		BytesConcat, ///< .concat() on bytes (type type)
+		StringConcat, ///< .concat() on string (type type)
 		ObjectCreation, ///< array creation using new
 		Assert, ///< assert()
 		Require, ///< require()

libsolidity/codegen/ExpressionCompiler.cpp

@@ -1101,6 +1101,7 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall)
 			ArrayUtils(m_context).popStorageArrayElement(*arrayType);
 			break;
 		}
+		case FunctionType::Kind::StringConcat:
 		case FunctionType::Kind::BytesConcat:
 		{
 			_functionCall.expression().accept(*this);
@@ -1121,8 +1122,16 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall)
 				else
 				{
 					solAssert(!dynamic_cast<RationalNumberType const*>(argument->annotation().type), "");
-					solAssert(argument->annotation().type->isImplicitlyConvertibleTo(*TypeProvider::bytesMemory()), "");
-					targetTypes.emplace_back(TypeProvider::bytesMemory());
+					if (function.kind() == FunctionType::Kind::StringConcat)
+					{
+						solAssert(argument->annotation().type->isImplicitlyConvertibleTo(*TypeProvider::stringMemory()), "");
+						targetTypes.emplace_back(TypeProvider::stringMemory());
+					}
+					else if (function.kind() == FunctionType::Kind::BytesConcat)
+					{
+						solAssert(argument->annotation().type->isImplicitlyConvertibleTo(*TypeProvider::bytesMemory()), "");
+						targetTypes.emplace_back(TypeProvider::bytesMemory());
+					}
 				}
 			}
 			utils().fetchFreeMemoryPointer();

libsolidity/codegen/YulUtilFunctions.cpp

@@ -2475,18 +2475,26 @@ string YulUtilFunctions::copyArrayFromStorageToMemoryFunction(ArrayType const& _
 	});
 }
 
-string YulUtilFunctions::bytesConcatFunction(vector<Type const*> const& _argumentTypes)
+string YulUtilFunctions::bytesOrStringConcatFunction(
+	vector<Type const*> const& _argumentTypes,
+	FunctionType::Kind _functionTypeKind
+)
 {
-	string functionName = "bytes_concat";
+	solAssert(_functionTypeKind == FunctionType::Kind::BytesConcat || _functionTypeKind == FunctionType::Kind::StringConcat);
+	std::string functionName = (_functionTypeKind == FunctionType::Kind::StringConcat) ? "string_concat" : "bytes_concat";
 	size_t totalParams = 0;
 	vector<Type const*> targetTypes;
+
 	for (Type const* argumentType: _argumentTypes)
 	{
-		solAssert(
-			argumentType->isImplicitlyConvertibleTo(*TypeProvider::bytesMemory()) ||
-			argumentType->isImplicitlyConvertibleTo(*TypeProvider::fixedBytes(32)),
-			""
-		);
+		if (_functionTypeKind == FunctionType::Kind::StringConcat)
+			solAssert(argumentType->isImplicitlyConvertibleTo(*TypeProvider::stringMemory()));
+		else if (_functionTypeKind == FunctionType::Kind::BytesConcat)
+			solAssert(
+				argumentType->isImplicitlyConvertibleTo(*TypeProvider::bytesMemory()) ||
+				argumentType->isImplicitlyConvertibleTo(*TypeProvider::fixedBytes(32))
+			);
+
 		if (argumentType->category() == Type::Category::FixedBytes)
 			targetTypes.emplace_back(argumentType);
 		else if (
@@ -2496,15 +2504,16 @@ string YulUtilFunctions::bytesConcatFunction(vector<Type const*> const& _argumen
 			targetTypes.emplace_back(TypeProvider::fixedBytes(static_cast<unsigned>(literalType->value().size())));
 		else
 		{
-			solAssert(!dynamic_cast<RationalNumberType const*>(argumentType), "");
-			solAssert(argumentType->isImplicitlyConvertibleTo(*TypeProvider::bytesMemory()), "");
-			targetTypes.emplace_back(TypeProvider::bytesMemory());
+			solAssert(!dynamic_cast<RationalNumberType const*>(argumentType));
+			targetTypes.emplace_back(
+				_functionTypeKind == FunctionType::Kind::StringConcat ?
+				TypeProvider::stringMemory() :
+				TypeProvider::bytesMemory()
+			);
 		}
-
 		totalParams += argumentType->sizeOnStack();
 		functionName += "_" + argumentType->identifier();
 	}
-
 	return m_functionCollector.createFunction(functionName, [&]() {
 		Whiskers templ(R"(
 			function <functionName>(<parameters>) -> outPtr {