Implement string.concat() and bytes.concat() (#1590)

This is what solc implements. Solang has implement a + b for string
concatenation, which this PR removes.

Fixes: https://github.com/hyperledger/solang/issues/1558

Signed-off-by: Sean Young <sean@mess.org>
Co-authored-by: Lucas Steuernagel <38472950+LucasSte@users.noreply.github.com>

CHANGELOG.md

@@ -2,6 +2,15 @@
 All notable changes to [Solang](https://github.com/hyperledger/solang/)
 will be documented here.
 
+## Unreleased
+
+### Added
+- The `string.concat()` and `bytes.concat()` builtin functions are supported. [seanyoung](https://github.com/seanyoung)
+
+### Changed
+- **BREAKING** The non-standard extension of concatenating strings using the `+` operator
+  has been removed, use `string.concat()` instead. [seanyoung](https://github.com/seanyoung)
+
 ## v0.3.3 Atlantis
 
 This release improves the Solana developer experience, since now required

docs/examples/solana/contract_call.sol

@@ -16,6 +16,6 @@ contract Math {
 
 contract English {
     function concatenate(string a, string b) external returns (string) {
-        return a + b;
+        return string.concat(a, b);
     }
-}
+}

docs/examples/string_type.sol

@@ -1,6 +1,6 @@
 contract example {
     function test1(string s) public returns (bool) {
-        string str = "Hello, " + s + "!";
+        string str = string.concat("Hello, ", s, "!");
 
         return (str == "Hello, World!");
     }

docs/examples/tags.sol

@@ -6,6 +6,6 @@
 contract c {
     /// @param name The name which will be greeted
     function say_hello(string name) public {
-        print("Hello, " + name + "!");
+        print(string.concat("Hello, ", name, "!"));
     }
 }

integration/polkadot/external_call.sol

@@ -55,6 +55,6 @@ contract callee2 {
     }
 
     function do_stuff2(string x) public pure returns (string) {
-        return "x:" + x;
+        return string.concat("x:", x);
     }
-}
+}

integration/solana/external_call.sol

@@ -65,6 +65,6 @@ contract callee2 {
     }
 
     function do_stuff2(string x) public pure returns (string) {
-        return "x:" + x;
+        return string.concat("x:", x);
     }
-}
+}

src/bin/languageserver/mod.rs

@@ -1057,14 +1057,6 @@ impl<'a> Builder<'a> {
                     self.expression(expr, symtab);
                 }
             }
-            ast::Expression::StringConcat { left, right, .. } => {
-                if let ast::StringLocation::RunTime(expr) = left {
-                    self.expression(expr, symtab);
-                }
-                if let ast::StringLocation::RunTime(expr) = right {
-                    self.expression(expr, symtab);
-                }
-            }
 
             ast::Expression::InternalFunction {loc, function_no, ..} => {
                 let fnc = &self.ns.functions[*function_no];

src/codegen/cfg.rs

@@ -925,11 +925,6 @@ impl ControlFlowGraph {
                 self.location_to_string(contract, ns, left),
                 self.location_to_string(contract, ns, right)
             ),
-            Expression::StringConcat { left, right, .. } => format!(
-                "(concat ({}) ({}))",
-                self.location_to_string(contract, ns, left),
-                self.location_to_string(contract, ns, right)
-            ),
             Expression::Keccak256 { exprs, .. } => format!(
                 "(keccak256 {})",
                 exprs

src/codegen/constant_folding.rs

@@ -616,12 +616,12 @@ fn expression(
         Expression::StringCompare { loc, left, right } => {
             string_compare(loc, left, right, vars, cfg, ns)
         }
-        Expression::StringConcat {
+        Expression::Builtin {
             loc,
-            ty,
-            left,
-            right,
-        } => string_concat(loc, ty, left, right, vars, cfg, ns),
+            kind: Builtin::Concat,
+            args,
+            ..
+        } => bytes_concat(loc, args, vars, cfg, ns),
         Expression::Builtin {
             loc,
             tys,
@@ -1628,15 +1628,43 @@ fn bytes_cast(
 ) -> (Expression, bool) {
     let (expr, _) = expression(expr, vars, cfg, ns);
 
-    (
-        Expression::BytesCast {
-            loc: *loc,
-            ty: to.clone(),
-            from: from.clone(),
-            expr: Box::new(expr),
-        },
-        false,
-    )
+    if let Expression::NumberLiteral {
+        loc,
+        ty: Type::Bytes(len),
+        value,
+    } = expr
+    {
+        let (_, mut bs) = value.to_bytes_be();
+
+        while bs.len() < len as usize {
+            bs.insert(0, 0);
+        }
+
+        (
+            Expression::AllocDynamicBytes {
+                loc,
+                ty: Type::DynamicBytes,
+                size: Expression::NumberLiteral {
+                    loc,
+                    ty: Type::Uint(32),
+                    value: len.into(),
+                }
+                .into(),
+                initializer: Some(bs),
+            },
+            false,
+        )
+    } else {
+        (
+            Expression::BytesCast {
+                loc: *loc,
+                ty: to.clone(),
+                from: from.clone(),
+                expr: Box::new(expr),
+            },
+            false,
+        )
+    }
 }
 
 fn more(
@@ -1924,51 +1952,90 @@ fn string_compare(
     }
 }
 
-fn string_concat(
+fn bytes_concat(
     loc: &pt::Loc,
-    ty: &Type,
-    left: &StringLocation<Expression>,
-    right: &StringLocation<Expression>,
+    args: &[Expression],
     vars: Option<&reaching_definitions::VarDefs>,
     cfg: &ControlFlowGraph,
     ns: &mut Namespace,
 ) -> (Expression, bool) {
-    if let (StringLocation::CompileTime(left), StringLocation::CompileTime(right)) = (left, right) {
-        let mut bs = Vec::with_capacity(left.len() + right.len());
+    let mut last = None;
+    let mut res = Vec::new();
 
-        bs.extend(left);
-        bs.extend(right);
+    for arg in args {
+        let expr = expression(arg, vars, cfg, ns).0;
 
-        (
-            Expression::BytesLiteral {
-                loc: *loc,
-                ty: ty.clone(),
-                value: bs,
-            },
-            true,
-        )
-    } else {
-        let left = if let StringLocation::RunTime(left) = left {
-            StringLocation::RunTime(Box::new(expression(left, vars, cfg, ns).0))
+        if let Expression::AllocDynamicBytes {
+            initializer: Some(bs),
+            ..
+        } = &expr
+        {
+            if bs.is_empty() {
+                continue;
+            }
+
+            if let Some(Expression::AllocDynamicBytes {
+                size,
+                initializer: Some(init),
+                ..
+            }) = &mut last
+            {
+                let Expression::NumberLiteral { value, .. } = size.as_mut() else {
+                    unreachable!();
+                };
+
+                *value += bs.len();
+
+                init.extend_from_slice(bs);
+            } else {
+                last = Some(expr);
+            }
         } else {
-            left.clone()
-        };
+            if let Some(expr) = last {
+                res.push(expr);
+                last = None;
+            }
+            res.push(expr);
+        }
+    }
 
-        let right = if let StringLocation::RunTime(right) = right {
-            StringLocation::RunTime(Box::new(expression(right, vars, cfg, ns).0))
+    if res.is_empty() {
+        if let Some(expr) = last {
+            (expr, false)
         } else {
-            right.clone()
-        };
+            (
+                Expression::AllocDynamicBytes {
+                    loc: *loc,
+                    ty: Type::DynamicBytes,
+                    size: Expression::NumberLiteral {
+                        loc: *loc,
+                        ty: Type::Uint(32),
+                        value: 0.into(),
+                    }
+                    .into(),
+                    initializer: None,
+                },
+                false,
+            )
+        }
+    } else {
+        if let Some(expr) = last {
+            res.push(expr);
+        }
 
-        (
-            Expression::StringConcat {
-                loc: *loc,
-                ty: ty.clone(),
-                left,
-                right,
-            },
-            false,
-        )
+        if res.len() == 1 {
+            (res[0].clone(), false)
+        } else {
+            (
+                Expression::Builtin {
+                    loc: *loc,
+                    tys: vec![Type::DynamicBytes],
+                    kind: Builtin::Concat,
+                    args: res,
+                },
+                false,
+            )
+        }
     }
 }
 

src/codegen/events/polkadot.rs

@@ -9,7 +9,7 @@ use crate::codegen::events::EventEmitter;
 use crate::codegen::expression::expression;
 use crate::codegen::vartable::Vartable;
 use crate::codegen::{Builtin, Expression, Options};
-use crate::sema::ast::{self, Function, Namespace, RetrieveType, StringLocation, Type};
+use crate::sema::ast::{self, Function, Namespace, RetrieveType, Type};
 use ink_env::hash::{Blake2x256, CryptoHash};
 use parity_scale_codec::Encode;
 use solang_parser::pt;
@@ -124,13 +124,23 @@ impl EventEmitter for PolkadotEventEmitter<'_> {
             }
 
             let encoded = abi_encode(&loc, vec![value], self.ns, vartab, cfg, false).0;
-            let prefix = StringLocation::CompileTime(topic_prefixes.pop_front().unwrap());
-            let value = StringLocation::RunTime(encoded.into());
-            let concatenated = Expression::StringConcat {
+            let first_prefix = topic_prefixes.pop_front().unwrap();
+            let prefix = Expression::AllocDynamicBytes {
                 loc,
-                ty: Type::DynamicBytes,
-                left: prefix,
-                right: value,
+                ty: Type::Slice(Type::Bytes(1).into()),
+                size: Expression::NumberLiteral {
+                    loc,
+                    ty: Type::Uint(32),
+                    value: first_prefix.len().into(),
+                }
+                .into(),
+                initializer: Some(first_prefix),
+            };
+            let concatenated = Expression::Builtin {
+                loc,
+                kind: Builtin::Concat,
+                tys: vec![Type::DynamicBytes],
+                args: vec![prefix, encoded],
             };
 
             vartab.new_dirty_tracker();

src/codegen/expression.rs

@@ -731,17 +731,6 @@ pub fn expression(
             left: string_location(left, cfg, contract_no, func, ns, vartab, opt),
             right: string_location(right, cfg, contract_no, func, ns, vartab, opt),
         },
-        ast::Expression::StringConcat {
-            loc,
-            ty,
-            left,
-            right,
-        } => Expression::StringConcat {
-            loc: *loc,
-            ty: ty.clone(),
-            left: string_location(left, cfg, contract_no, func, ns, vartab, opt),
-            right: string_location(right, cfg, contract_no, func, ns, vartab, opt),
-        },
         ast::Expression::Or { loc, left, right } => {
             expr_or(left, cfg, contract_no, func, ns, vartab, loc, right, opt)
         }

src/codegen/mod.rs

@@ -609,12 +609,6 @@ pub enum Expression {
         left: StringLocation<Expression>,
         right: StringLocation<Expression>,
     },
-    StringConcat {
-        loc: pt::Loc,
-        ty: Type,
-        left: StringLocation<Expression>,
-        right: StringLocation<Expression>,
-    },
     StructLiteral {
         loc: pt::Loc,
         ty: Type,
@@ -712,7 +706,6 @@ impl CodeLocation for Expression {
             | Expression::ConstArrayLiteral { loc, .. }
             | Expression::StructMember { loc, .. }
             | Expression::StringCompare { loc, .. }
-            | Expression::StringConcat { loc, .. }
             | Expression::FunctionArg { loc, .. }
             | Expression::ShiftRight { loc, .. }
             | Expression::ShiftLeft { loc, .. }
@@ -804,8 +797,7 @@ impl Recurse for Expression {
                 }
             }
 
-            Expression::StringCompare { left, right, .. }
-            | Expression::StringConcat { left, right, .. } => {
+            Expression::StringCompare { left, right, .. } => {
                 if let StringLocation::RunTime(exp) = left {
                     exp.recurse(cx, f);
                 }
@@ -859,7 +851,6 @@ impl RetrieveType for Expression {
             | Expression::ArrayLiteral { ty, .. }
             | Expression::ConstArrayLiteral { ty, .. }
             | Expression::StructMember { ty, .. }
-            | Expression::StringConcat { ty, .. }
             | Expression::FunctionArg { ty, .. }
             | Expression::AllocDynamicBytes { ty, .. }
             | Expression::BytesCast { ty, .. }
@@ -1655,27 +1646,6 @@ impl Expression {
                         }
                     },
                 },
-                Expression::StringConcat {
-                    loc,
-                    ty,
-                    left,
-                    right,
-                } => Expression::StringConcat {
-                    loc: *loc,
-                    ty: ty.clone(),
-                    left: match left {
-                        StringLocation::CompileTime(_) => left.clone(),
-                        StringLocation::RunTime(expr) => {
-                            StringLocation::RunTime(Box::new(filter(expr, ctx)))
-                        }
-                    },
-                    right: match right {
-                        StringLocation::CompileTime(_) => right.clone(),
-                        StringLocation::RunTime(expr) => {
-                            StringLocation::RunTime(Box::new(filter(expr, ctx)))
-                        }
-                    },
-                },
                 Expression::FormatString { loc, args } => {
                     let args = args.iter().map(|(f, e)| (*f, filter(e, ctx))).collect();
 
@@ -1792,6 +1762,7 @@ pub enum Builtin {
     WriteUint128LE,
     WriteUint256LE,
     WriteBytes,
+    Concat,
 }
 
 impl From<&ast::Builtin> for Builtin {
@@ -1853,6 +1824,7 @@ impl From<&ast::Builtin> for Builtin {
             ast::Builtin::BaseFee => Builtin::BaseFee,
             ast::Builtin::PrevRandao => Builtin::PrevRandao,
             ast::Builtin::ContractCode => Builtin::ContractCode,
+            ast::Builtin::StringConcat | ast::Builtin::BytesConcat => Builtin::Concat,
             _ => panic!("Builtin should not be in the cfg"),
         }
     }

src/codegen/subexpression_elimination/available_expression_set.rs

@@ -273,8 +273,7 @@ impl<'a, 'b: 'a> AvailableExpressionSet<'a> {
                 return Some(exp_id);
             }
 
-            Expression::StringCompare { left, right, .. }
-            | Expression::StringConcat { left, right, .. } => {
+            Expression::StringCompare { left, right, .. } => {
                 return if let (
                     StringLocation::RunTime(operand_1),
                     StringLocation::RunTime(operand_2),
@@ -398,8 +397,7 @@ impl<'a, 'b: 'a> AvailableExpressionSet<'a> {
                 return self.expr_map.get(&key).copied();
             }
 
-            Expression::StringCompare { left, right, .. }
-            | Expression::StringConcat { left, right, .. } => {
+            Expression::StringCompare { left, right, .. } => {
                 if let (StringLocation::RunTime(operand_1), StringLocation::RunTime(operand_2)) =
                     (left, right)
                 {
@@ -513,7 +511,7 @@ impl<'a, 'b: 'a> AvailableExpressionSet<'a> {
             }
 
             Expression::StringCompare { loc: _, left, right }
-            | Expression::StringConcat {  left, right, .. } => {
+            => {
                 if let (StringLocation::RunTime(operand_1), StringLocation::RunTime(operand_2)) =
                     (left, right)
                 {

src/codegen/subexpression_elimination/expression.rs

@@ -209,27 +209,6 @@ impl Expression {
                 }
             }
 
-            Expression::StringConcat {
-                loc,
-                ty: expr_type,
-                left: left_exp,
-                right: right_exp,
-            } => {
-                if !matches!(
-                    (left_exp, right_exp),
-                    (StringLocation::RunTime(_), StringLocation::RunTime(_))
-                ) {
-                    unreachable!("String concat operation does not contain runtime argumetns")
-                }
-
-                Expression::StringConcat {
-                    loc: *loc,
-                    ty: expr_type.clone(),
-                    left: StringLocation::RunTime(Box::new(left.clone())),
-                    right: StringLocation::RunTime(Box::new(right.clone())),
-                }
-            }
-
             _ => unreachable!("Cannot rebuild this expression"),
         }
     }

src/codegen/subexpression_elimination/operator.rs

@@ -117,7 +117,6 @@ impl Expression {
             Expression::NotEqual { .. } => Operator::NotEqual,
             Expression::BitwiseNot { .. } => Operator::BitwiseNot,
             Expression::StringCompare { .. } => Operator::StringCompare,
-            Expression::StringConcat { .. } => Operator::StringConcat,
             Expression::AdvancePointer { .. } => Operator::AdvancePointer,
             _ => {
                 unreachable!("Expression does not represent an operator.")

src/codegen/subexpression_elimination/tests.rs

@@ -397,24 +397,12 @@ fn string() {
 
     let op3 = StringLocation::CompileTime(vec![0, 1]);
 
-    let concat = Expression::StringConcat {
-        loc: Loc::Codegen,
-        ty: Type::String,
-        left: op1.clone(),
-        right: op2.clone(),
-    };
     let compare = Expression::StringCompare {
         loc: Loc::Codegen,
         left: op2.clone(),
         right: op1.clone(),
     };
 
-    let concat2 = Expression::StringConcat {
-        loc: Loc::Codegen,
-        ty: Type::String,
-        left: op2.clone(),
-        right: op1,
-    };
     let compare2 = Expression::StringCompare {
         loc: Loc::Codegen,
         left: op2,
@@ -426,10 +414,10 @@ fn string() {
         res: 0,
         contract_no: 0,
         constructor_no: None,
-        encoded_args: concat.clone(),
-        value: Some(compare.clone()),
-        gas: concat2.clone(),
-        salt: Some(compare2.clone()),
+        encoded_args: compare.clone(),
+        value: None,
+        gas: compare2.clone(),
+        salt: None,
         address: None,
         seeds: None,
         loc: Loc::Codegen,
@@ -442,9 +430,7 @@ fn string() {
 
     set.process_instruction(&instr, &mut ave, &mut Some(&mut cst));
 
-    assert!(set.find_expression(&concat).is_some());
     assert!(set.find_expression(&compare).is_some());
-    assert!(set.find_expression(&concat2).is_some());
     assert!(set.find_expression(&compare2).is_none());
 
     assert!(set.find_expression(&var1).is_some());

src/emit/expression.rs

@@ -31,6 +31,8 @@ pub(super) fn expression<'a, T: TargetRuntime<'a> + ?Sized>(
     function: FunctionValue<'a>,
     ns: &Namespace,
 ) -> BasicValueEnum<'a> {
+    emit_context!(bin);
+
     match e {
         Expression::FunctionArg { arg_no, .. } => function.get_nth_param(*arg_no as u32).unwrap(),
         Expression::BoolLiteral { value, .. } => bin
@@ -1550,20 +1552,6 @@ pub(super) fn expression<'a, T: TargetRuntime<'a> + ?Sized>(
                 .left()
                 .unwrap()
         }
-        Expression::StringConcat { left, right, .. } => {
-            let (left, left_len) = string_location(target, bin, left, vartab, function, ns);
-            let (right, right_len) = string_location(target, bin, right, vartab, function, ns);
-
-            bin.builder
-                .build_call(
-                    bin.module.get_function("concat").unwrap(),
-                    &[left.into(), left_len.into(), right.into(), right_len.into()],
-                    "",
-                )
-                .try_as_basic_value()
-                .left()
-                .unwrap()
-        }
         Expression::ReturnData { .. } => target.return_data(bin, function).into(),
         Expression::StorageArrayLength { array, elem_ty, .. } => {
             let slot = expression(target, bin, array, vartab, function, ns).into_int_value();
@@ -1816,6 +1804,84 @@ pub(super) fn expression<'a, T: TargetRuntime<'a> + ?Sized>(
                 )
                 .into()
         }
+        Expression::Builtin {
+            kind: Builtin::Concat,
+            args,
+            ..
+        } => {
+            let vector_ty = bin.module.get_struct_type("struct.vector").unwrap();
+
+            let mut length = i32_zero!();
+
+            let args: Vec<_> = args
+                .iter()
+                .map(|arg| {
+                    let v = expression(target, bin, arg, vartab, function, ns);
+
+                    length = bin
+                        .builder
+                        .build_int_add(length, bin.vector_len(v), "length");
+
+                    v
+                })
+                .collect();
+
+            let size = bin.builder.build_int_add(
+                length,
+                vector_ty
+                    .size_of()
+                    .unwrap()
+                    .const_cast(bin.context.i32_type(), false),
+                "size",
+            );
+
+            let v = bin
+                .builder
+                .build_call(
+                    bin.module.get_function("__malloc").unwrap(),
+                    &[size.into()],
+                    "",
+                )
+                .try_as_basic_value()
+                .left()
+                .unwrap()
+                .into_pointer_value();
+
+            let mut dest = bin.vector_bytes(v.into());
+
+            for arg in args {
+                let from = bin.vector_bytes(arg);
+                let len = bin.vector_len(arg);
+
+                dest = bin
+                    .builder
+                    .build_call(
+                        bin.module.get_function("__memcpy").unwrap(),
+                        &[dest.into(), from.into(), len.into()],
+                        "",
+                    )
+                    .try_as_basic_value()
+                    .left()
+                    .unwrap()
+                    .into_pointer_value();
+            }
+
+            // Update the len and size field of the vector struct
+            let len_ptr = bin
+                .builder
+                .build_struct_gep(vector_ty, v, 0, "len")
+                .unwrap();
+            bin.builder.build_store(len_ptr, length);
+
+            let size_ptr = bin
+                .builder
+                .build_struct_gep(vector_ty, v, 1, "size")
+                .unwrap();
+
+            bin.builder.build_store(size_ptr, length);
+
+            v.into()
+        }
         Expression::Builtin { .. } => target.builtin(bin, e, vartab, function, ns),
         Expression::InternalFunctionCfg { cfg_no, .. } => bin.functions[cfg_no]
             .as_global_value()

src/sema/ast.rs

@@ -1117,12 +1117,6 @@ pub enum Expression {
         left: StringLocation<Expression>,
         right: StringLocation<Expression>,
     },
-    StringConcat {
-        loc: pt::Loc,
-        ty: Type,
-        left: StringLocation<Expression>,
-        right: StringLocation<Expression>,
-    },
 
     Or {
         loc: pt::Loc,
@@ -1375,8 +1369,7 @@ impl Recurse for Expression {
 
                 Expression::AllocDynamicBytes { length, .. } => length.recurse(cx, f),
                 Expression::StorageArrayLength { array, .. } => array.recurse(cx, f),
-                Expression::StringCompare { left, right, .. }
-                | Expression::StringConcat { left, right, .. } => {
+                Expression::StringCompare { left, right, .. } => {
                     if let StringLocation::RunTime(expr) = left {
                         expr.recurse(cx, f);
                     }
@@ -1504,7 +1497,6 @@ impl CodeLocation for Expression {
             | Expression::AllocDynamicBytes { loc, .. }
             | Expression::StorageArrayLength { loc, .. }
             | Expression::StringCompare { loc, .. }
-            | Expression::StringConcat { loc, .. }
             | Expression::InternalFunction { loc, .. }
             | Expression::ExternalFunction { loc, .. }
             | Expression::InternalFunctionCall { loc, .. }
@@ -1707,6 +1699,8 @@ pub enum Builtin {
     UserTypeWrap,
     UserTypeUnwrap,
     ECRecover,
+    StringConcat,
+    BytesConcat,
 }
 
 #[derive(PartialEq, Eq, Clone, Debug)]

src/sema/builtin.rs

@@ -33,7 +33,7 @@ pub struct Prototype {
 }
 
 // A list of all Solidity builtins functions
-static BUILTIN_FUNCTIONS: Lazy<[Prototype; 25]> = Lazy::new(|| {
+static BUILTIN_FUNCTIONS: Lazy<[Prototype; 27]> = Lazy::new(|| {
     [
         Prototype {
             builtin: Builtin::Assert,
@@ -322,6 +322,28 @@ static BUILTIN_FUNCTIONS: Lazy<[Prototype; 25]> = Lazy::new(|| {
             doc: "Recover the address associated with the public key from elliptic curve signature",
             constant: false,
         },
+        Prototype {
+            builtin: Builtin::StringConcat,
+            namespace: Some("string"),
+            method: vec![],
+            name: "concat",
+            params: vec![Type::String, Type::String],
+            ret: vec![Type::String],
+            target: vec![],
+            doc: "Concatenate string",
+            constant: true,
+        },
+        Prototype {
+            builtin: Builtin::BytesConcat,
+            namespace: Some("bytes"),
+            method: vec![],
+            name: "concat",
+            params: vec![Type::DynamicBytes, Type::DynamicBytes],
+            ret: vec![Type::DynamicBytes],
+            target: vec![],
+            doc: "Concatenate bytes",
+            constant: true,
+        },
     ]
 });
 
@@ -1044,8 +1066,38 @@ pub(super) fn resolve_namespace_call(
     symtable: &mut Symtable,
     diagnostics: &mut Diagnostics,
 ) -> Result<Expression, ()> {
+    if name == "concat" {
+        let (kind, ty) = match namespace {
+            "string" => (Builtin::StringConcat, Type::String),
+            "bytes" => (Builtin::BytesConcat, Type::DynamicBytes),
+            _ => unreachable!(),
+        };
+
+        let mut resolved_args = Vec::new();
+
+        for arg in args {
+            let expr = expression(
+                arg,
+                context,
+                ns,
+                symtable,
+                diagnostics,
+                ResolveTo::Type(&ty),
+            )?;
+
+            resolved_args.push(expr.cast(loc, &ty, true, ns, diagnostics)?);
+        }
+
+        return Ok(Expression::Builtin {
+            loc: *loc,
+            tys: vec![ty],
+            kind,
+            args: resolved_args,
+        });
+    }
+
     // The abi.* functions need special handling, others do not
-    if namespace != "abi" {
+    if namespace != "abi" && namespace != "string" {
         return resolve_call(
             loc,
             Some(namespace),

src/sema/dotgraphviz.rs

@@ -1146,28 +1146,6 @@ impl Dot {
                 self.add_string_location(left, func, ns, node, String::from("left"));
                 self.add_string_location(right, func, ns, node, String::from("right"));
             }
-            Expression::StringConcat {
-                loc,
-                ty,
-                left,
-                right,
-            } => {
-                let node = self.add_node(
-                    Node::new(
-                        "string_concat",
-                        vec![
-                            format!("string concat {}", ty.to_string(ns)),
-                            ns.loc_to_string(PathDisplay::FullPath, loc),
-                        ],
-                    ),
-                    Some(parent),
-                    Some(parent_rel),
-                );
-
-                self.add_string_location(left, func, ns, node, String::from("left"));
-                self.add_string_location(right, func, ns, node, String::from("right"));
-            }
-
             Expression::Or { loc, left, right } => {
                 let labels = vec![
                     String::from("logical or"),

src/sema/expression/arithmetic.rs

@@ -747,62 +747,27 @@ pub(super) fn addition(
         return Ok(expr);
     }
 
-    // Concatenate stringliteral with stringliteral
-    if let (Expression::BytesLiteral { value: l, .. }, Expression::BytesLiteral { value: r, .. }) =
-        (&left, &right)
-    {
-        let mut c = Vec::with_capacity(l.len() + r.len());
-        c.extend_from_slice(l);
-        c.extend_from_slice(r);
-        let length = c.len();
-        return Ok(Expression::BytesLiteral {
-            loc: *loc,
-            ty: Type::Bytes(length as u8),
-            value: c,
-        });
-    }
-
     let left_type = left.ty();
     let right_type = right.ty();
 
-    // compare string against literal
-    match (&left, &right_type) {
-        (Expression::BytesLiteral { value, .. }, Type::String)
-        | (Expression::BytesLiteral { value, .. }, Type::DynamicBytes) => {
-            return Ok(Expression::StringConcat {
-                loc: *loc,
-                ty: right_type,
-                left: StringLocation::CompileTime(value.clone()),
-                right: StringLocation::RunTime(Box::new(right)),
-            });
-        }
-        _ => {}
-    }
-
-    match (&right, &left_type) {
-        (Expression::BytesLiteral { value, .. }, Type::String)
-        | (Expression::BytesLiteral { value, .. }, Type::DynamicBytes) => {
-            return Ok(Expression::StringConcat {
-                loc: *loc,
-                ty: left_type,
-                left: StringLocation::RunTime(Box::new(left)),
-                right: StringLocation::CompileTime(value.clone()),
-            });
-        }
-        _ => {}
-    }
-
-    // compare string
+    // Solang 0.3.3 and earlier supported + for concatenating strings/bytes. Give a specific error
+    // saying this must be done using string.concat() and bytes.concat() builtin.
     match (&left_type, &right_type) {
-        (Type::String, Type::String) | (Type::DynamicBytes, Type::DynamicBytes) => {
-            return Ok(Expression::StringConcat {
-                loc: *loc,
-                ty: right_type,
-                left: StringLocation::RunTime(Box::new(left)),
-                right: StringLocation::RunTime(Box::new(right)),
-            });
+        (Type::DynamicBytes | Type::Bytes(_), Type::DynamicBytes | Type::Bytes(_)) => {
+            diagnostics.push(Diagnostic::error(
+                *loc,
+                "concatenate bytes using the builtin bytes.concat(a, b)".into(),
+            ));
+            return Err(());
         }
-        _ => {}
+        (Type::String, Type::String) => {
+            diagnostics.push(Diagnostic::error(
+                *loc,
+                "concatenate string using the builtin string.concat(a, b)".into(),
+            ));
+            return Err(());
+        }
+        _ => (),
     }
 
     let ty = coerce_number(

src/sema/expression/function_call.rs

@@ -516,7 +516,20 @@ fn try_namespace(
     diagnostics: &mut Diagnostics,
     resolve_to: ResolveTo,
 ) -> Result<Option<Expression>, ()> {
-    if let pt::Expression::Variable(namespace) = var {
+    let namespace = match var {
+        pt::Expression::Variable(namespace) => Some(namespace.clone()),
+        pt::Expression::Type(loc, pt::Type::String) => Some(pt::Identifier {
+            name: "string".to_owned(),
+            loc: *loc,
+        }),
+        pt::Expression::Type(loc, pt::Type::DynamicBytes) => Some(pt::Identifier {
+            name: "bytes".to_owned(),
+            loc: *loc,
+        }),
+        _ => None,
+    };
+
+    if let Some(namespace) = &namespace {
         if builtin::is_builtin_call(Some(&namespace.name), &func.name, ns) {
             if let Some(loc) = call_args_loc {
                 diagnostics.push(Diagnostic::error(

src/sema/expression/retrieve_type.rs

@@ -17,8 +17,7 @@ impl RetrieveType for Expression {
             | Expression::Not { .. }
             | Expression::StringCompare { .. } => Type::Bool,
             Expression::CodeLiteral { .. } => Type::DynamicBytes,
-            Expression::StringConcat { ty, .. }
-            | Expression::BytesLiteral { ty, .. }
+            Expression::BytesLiteral { ty, .. }
             | Expression::NumberLiteral { ty, .. }
             | Expression::RationalNumberLiteral { ty, .. }
             | Expression::StructLiteral { ty, .. }

stdlib/stdlib.c

@@ -43,7 +43,7 @@ void __memcpy8(void *_dest, void *_src, uint32_t length)
     } while (--length);
 }
 
-void __memcpy(void *_dest, const void *_src, uint32_t length)
+void *__memcpy(void *_dest, const void *_src, uint32_t length)
 {
     uint8_t *dest = _dest;
     const uint8_t *src = _src;
@@ -52,6 +52,8 @@ void __memcpy(void *_dest, const void *_src, uint32_t length)
     {
         *dest++ = *src++;
     }
+
+    return dest;
 }
 
 /*
@@ -191,26 +193,4 @@ struct vector *vector_new(uint32_t members, uint32_t size, uint8_t *initial)
     return v;
 }
 
-struct vector *concat(uint8_t *left, uint32_t left_len, uint8_t *right, uint32_t right_len)
-{
-    uint32_t size_array = left_len + right_len;
-    struct vector *v = __malloc(sizeof(*v) + size_array);
-    v->len = size_array;
-    v->size = size_array;
-
-    uint8_t *data = v->data;
-
-    while (left_len--)
-    {
-        *data++ = *left++;
-    }
-
-    while (right_len--)
-    {
-        *data++ = *right++;
-    }
-
-    return v;
-}
-
-#endif
+#endif

stdlib/stdlib.h

@@ -12,5 +12,5 @@ struct vector
 
 extern void *__malloc(uint32_t size);
 extern void __memset(void *dest, uint8_t val, size_t length);
-extern void __memcpy(void *dest, const void *src, uint32_t length);
+extern void *__memcpy(void *dest, const void *src, uint32_t length);
 extern void __memcpy8(void *_dest, void *_src, uint32_t length);

tests/codegen.rs

@@ -34,11 +34,11 @@ fn run_test_for_path(path: &str) {
 
 #[derive(Debug)]
 enum Test {
-    Check(String),
-    CheckAbsent(String),
-    NotCheck(String),
-    Fail(String),
-    Rewind,
+    Check(usize, String),
+    CheckAbsent(usize, String),
+    NotCheck(usize, String),
+    Fail(usize, String),
+    Rewind(usize),
 }
 
 fn testcase(path: PathBuf) {
@@ -52,7 +52,7 @@ fn testcase(path: PathBuf) {
     let mut fails = Vec::new();
     let mut read_from = None;
 
-    for line in reader.lines() {
+    for (line_no, line) in reader.lines().enumerate() {
         let mut line = line.unwrap();
         line = line.trim().parse().unwrap();
         // The first line should be a command line (excluding "solang compile") after // RUN:
@@ -67,21 +67,21 @@ fn testcase(path: PathBuf) {
             read_from = Some(check.trim().to_string());
         // Read more input until you find a line that contains the needle // CHECK: needle
         } else if let Some(check) = line.strip_prefix("// CHECK:") {
-            checks.push(Test::Check(check.trim().to_string()));
+            checks.push(Test::Check(line_no, check.trim().to_string()));
         //
         } else if let Some(fail) = line.strip_prefix("// FAIL:") {
-            fails.push(Test::Fail(fail.trim().to_string()));
+            fails.push(Test::Fail(line_no, fail.trim().to_string()));
         // Ensure that the following line in the input does not match
         } else if let Some(not_check) = line.strip_prefix("// NOT-CHECK:") {
-            checks.push(Test::NotCheck(not_check.trim().to_string()));
+            checks.push(Test::NotCheck(line_no, not_check.trim().to_string()));
         // Check the output from here until the end of the file does not contain the needle
         } else if let Some(check_absent) = line.strip_prefix("// CHECK-ABSENT:") {
-            checks.push(Test::CheckAbsent(check_absent.trim().to_string()));
+            checks.push(Test::CheckAbsent(line_no, check_absent.trim().to_string()));
         // Go back to the beginning and find the needle from there, like // CHECK: but from
         // the beginning of the file.
         } else if let Some(check) = line.strip_prefix("// BEGIN-CHECK:") {
-            checks.push(Test::Rewind);
-            checks.push(Test::Check(check.trim().to_string()));
+            checks.push(Test::Rewind(line_no));
+            checks.push(Test::Check(line_no, check.trim().to_string()));
         }
     }
 
@@ -114,19 +114,19 @@ fn testcase(path: PathBuf) {
         let line = lines[current_line];
 
         match checks.get(current_check) {
-            Some(Test::Check(needle)) => {
+            Some(Test::Check(_, needle)) => {
                 if line.contains(needle) {
                     current_check += 1;
                 }
             }
-            Some(Test::NotCheck(needle)) => {
+            Some(Test::NotCheck(_, needle)) => {
                 if !line.contains(needle) {
                     current_check += 1;
                     // We should not advance line during a not check
                     current_line -= 1;
                 }
             }
-            Some(Test::CheckAbsent(needle)) => {
+            Some(Test::CheckAbsent(_, needle)) => {
                 for line in lines.iter().skip(current_line) {
                     if line.contains(needle) {
                         panic!(
@@ -138,7 +138,7 @@ fn testcase(path: PathBuf) {
                 }
                 current_check += 1;
             }
-            Some(Test::Rewind) => {
+            Some(Test::Rewind(_)) => {
                 current_line = 0;
                 current_check += 1;
                 continue;
@@ -146,7 +146,7 @@ fn testcase(path: PathBuf) {
             _ => (),
         }
 
-        if let Some(Test::Fail(needle)) = fails.get(current_fail) {
+        if let Some(Test::Fail(_, needle)) = fails.get(current_fail) {
             if line.contains(needle) {
                 current_fail += 1;
             }

tests/codegen_testcases/solidity/common_subexpression_elimination.sol

@@ -299,7 +299,7 @@ contract c1 {
     function test11(int a, int b) public returns (int) {
         string ast = "Hello!";
         string bst = "from Solang";
-        string cst = ast + bst;
+        string cst = string.concat(ast, bst);
         // CHECK: ty:int256 %1.cse_temp = (signed divide (arg #0) / (int256 2 * (arg #1)))
         // CHECK: call c1::c1::function::get__int256_int256 %1.cse_temp, (arg #1)
         int p = a + get(a/(2*b), b);
@@ -308,22 +308,22 @@ contract c1 {
         // CHECK: ty:bool %2.cse_temp = (strcmp (%ast) (%bst))
         // CHECK: branchcond %2.cse_temp, block2, block1
         bool e2 = e;
-        // CHECK: branchcond (strcmp (%cst) (%cst)), block3, block4
-        if (ast + bst == cst) {
+        // CHECK: branchcond (strcmp ((builtin Concat (%ast, %bst))) (%cst)), block3, block4
+        if (string.concat(ast, bst) == cst) {
             // CHECK: call c1::c1::function::get__int256_int256 %1.cse_temp, (arg #1)
             require(a + get(a/(2*b), b) < 0);
-            emit testEvent(a + get(a/(2*b) -p, b), p, ast+bst);
+            emit testEvent(a + get(a/(2*b) -p, b), p, string.concat(ast, bst));
         }
 
         // CHECK: branchcond %2.cse_temp, block21, block22
         if (ast == bst) {
-            ast = ast + "b";
+            ast = string.concat(ast, "b");
         }
         // CHECK: call c1::c1::function::get__int256_int256 (%1.cse_temp - %p), (arg #1)
 
         // CHECK: branchcond (strcmp (%ast) (%bst)), block24, block25
         while (ast == bst) {
-            ast = ast + "a";
+            ast = string.concat(ast, "a");
         }
 
         // CHECK: call c1::c1::function::get__int256_int256 (arg #1), (signed divide (arg #0) / (arg #1))
@@ -378,17 +378,17 @@ contract c1 {
         if(vec.length - (a+b) == 1) {
             // CHECK:  call c1::c1::function::testing__bytes %c
             string k = testing(bytes(c));
-            string p = "a" +k;
-            // CHECK: ty:string %p = (concat ((alloc string uint32 1 "a")) (%k))
+            string p = string.concat("a", k);
+            // CHECK: ty:string %p = (builtin Concat ((alloc string uint32 1 "a"), %k))
             // CHECK: branchcond ((builtin ArrayLength (%p)) == uint32 2), block11, block12
             if(p.length == 2) {
-                // CHECK: ty:string %p1 = (concat ((alloc string uint32 1 "a")) (%k))
-                string p1 = "a" + k;
+                // CHECK: ty:string %p1 = (builtin Concat ((alloc string uint32 1 "a"), %k))
+                string p1 = string.concat("a", k);
                 string l = p1;
             }
         }
 
-        // CHECK: branchcond (signed less (%a + (arg #1)) < int256 0), block14, block15
+        // CHECK: branchcond (signed less %2.cse_temp < int256 0), block14, block15
         while(a+b < 0) {
             // CHECK: branchcond (strcmp (%c) ("a")), block16, block17
             if("a" == c) {

tests/codegen_testcases/solidity/concat.sol

@@ -0,0 +1,15 @@
+// RUN: --target polkadot --emit cfg
+
+contract C {
+// BEGIN-CHECK: C::C::function::f1
+	function f1(string a) public returns (string) {
+		return string.concat("", a, "");
+		// CHECK: return (arg #0)
+	}
+
+// BEGIN-CHECK: C::C::function::f2
+	function f2(string a) public returns (string) {
+		return string.concat("b", "ar", ": ", a, "");
+		// CHECK: return (builtin Concat ((alloc string uint32 5 "bar: "), (arg #0)))
+	}
+}

tests/codegen_testcases/solidity/slice1.sol

@@ -41,7 +41,7 @@ contract c {
 			bool y = true;
 		}
 
-		string y = x + "if";
+		string y = string.concat(x, "if");
 
 		print(x);
 // CHECK: alloc slice bytes1 uint32 4 "foo4"
@@ -90,4 +90,4 @@ contract c {
 		// x modified via y
 // CHECK: alloc string uint32 4 "foo8"
 	}
-}
+}

tests/codegen_testcases/solidity/solana_bump.sol

@@ -9,7 +9,7 @@ contract C1 {
     }
     // BEGIN-CHECK: solang_dispatch
     // 25 must be the last seed in the call.
-    // CHECK: external call::regular address:address 0x0 payload:%instruction.temp.15 value:uint64 0 gas:uint64 0 accounts:%metas.temp.11 seeds:[1] [ [2] [ bytes(%my_seed), bytes(bytes from:bytes1 (bytes1 25)) ] ] contract|function:_ flags:
+    // CHECK: external call::regular address:address 0x0 payload:%instruction.temp.15 value:uint64 0 gas:uint64 0 accounts:%metas.temp.11 seeds:[1] [ [2] [ bytes(%my_seed), bytes((alloc bytes uint32 1 "\u{19}")) ] ] contract|function:_ flags:
 }
 
 contract C2 {
@@ -23,7 +23,7 @@ contract C2 {
     }
     // BEGIN-CHECK: solang_dispatch
     // 12 must be the last seed in the call.
-    // CHECK: external call::regular address:address 0x0 payload:%instruction.temp.27 value:uint64 0 gas:uint64 0 accounts:%metas.temp.23 seeds:[1] [ [4] [ (alloc slice bytes1 uint32 5 "apple"), (alloc slice bytes1 uint32 9 "pine_tree"), bytes(%my_seed), bytes(bytes from:bytes1 (bytes1 12)) ] ] contract|function:_ flags:
+    // CHECK: external call::regular address:address 0x0 payload:%instruction.temp.27 value:uint64 0 gas:uint64 0 accounts:%metas.temp.23 seeds:[1] [ [4] [ (alloc slice bytes1 uint32 5 "apple"), (alloc slice bytes1 uint32 9 "pine_tree"), bytes(%my_seed), bytes((alloc bytes uint32 1 "\u{c}")) ] ] contract|function:_ flags:
 }
 
 contract C3 {
@@ -37,4 +37,4 @@ contract C3 {
     // BEGIN-CHECK: solang_dispatch
     // bp must be the last seed in the call
     // CHECK: external call::regular address:address 0x0 payload:%instruction.temp.40 value:uint64 0 gas:uint64 0 accounts:%metas.temp.36 seeds:[1] [ [4] [ (alloc slice bytes1 uint32 9 "pineapple"), (alloc slice bytes1 uint32 7 "avocado"), bytes(%my_seed), bytes(bytes from:bytes1 (%bp)) ] ] contract|function:_ flags:
-}
+}

tests/contract_testcases/evm/concat.sol

@@ -0,0 +1,14 @@
+contract C {
+	function f1(bytes1 a, bytes b) public returns (bytes c) { c = a + b; }
+	function f2(bytes a, bytes2 b) public returns (bytes c) { c = a + b; }
+	function f3(bytes a, bytes b) public returns (bytes c) { c = a + b; }
+	function f4(string a, string b) public returns (string c) { c = a + b; }
+	function f(string a, bytes b) public returns (bytes c) { c = a + b; }
+}
+
+// ---- Expect: diagnostics ----
+// error: 2:64-69: concatenate bytes using the builtin bytes.concat(a, b)
+// error: 3:64-69: concatenate bytes using the builtin bytes.concat(a, b)
+// error: 4:63-68: concatenate bytes using the builtin bytes.concat(a, b)
+// error: 5:66-71: concatenate string using the builtin string.concat(a, b)
+// error: 6:63-64: expression of type string not allowed

tests/evm.rs

@@ -253,7 +253,7 @@ fn ethereum_solidity_tests() {
         })
         .sum();
 
-    assert_eq!(errors, 960);
+    assert_eq!(errors, 946);
 }
 
 fn set_file_contents(source: &str, path: &Path) -> (FileResolver, Vec<String>) {

tests/optimization_testcases/programs/a013e38f16b0d7f4fb05d2a127342a0a89e025b2.sol

@@ -4,7 +4,7 @@ contract c1 {
         string bst = "from Solang";
 
         while (ast == bst) {
-            ast = ast + "a";
+            ast = string.concat(ast, "a");
         }
 
         return ast;

tests/solana_tests/call.rs

@@ -14,7 +14,7 @@ fn simple_external_call() {
         r#"
         contract bar0 {
             function test_bar(string v) public {
-                print("bar0 says: " + v);
+                print(string.concat("bar0", " ", "says: ", v, ""));
             }
 
             @account(pid)
@@ -25,7 +25,7 @@ fn simple_external_call() {
 
         contract bar1 {
             function test_bar(string v) public {
-                print("bar1 says: " + v);
+                print(string.concat("bar1 says: ", v));
             }
         }"#,
     );

tests/solana_tests/create_contract.rs

@@ -24,7 +24,7 @@ fn simple_create_contract_no_seed() {
         contract bar1 {
             @payer(payer)
             constructor(string v) {
-                print("bar1 says: " + v);
+                print(string.concat("bar1 says: ", v));
             }
 
             function say_hello(string v) public {
@@ -105,7 +105,7 @@ fn simple_create_contract() {
         contract bar1 {
             @payer(pay)
             constructor(string v) {
-                print("bar1 says: " + v);
+                print(string.concat("bar1 says: ", v));
             }
 
             function say_hello(string v) public {
@@ -277,7 +277,7 @@ fn missing_contract() {
         @program_id("7vJKRaKLGCNUPuHWdeHCTknkYf3dHXXEZ6ri7dc6ngeV")
         contract bar1 {
             constructor(string v) {
-                print("bar1 says: " + v);
+                print(string.concat("bar1 says: ", v));
             }
 
             function say_hello(string v) public {
@@ -342,7 +342,7 @@ fn two_contracts() {
         contract bar1 {
             @payer(payer_account)
             constructor(string v) {
-                print("bar1 says: " + v);
+                print(string.concat("bar1 says: ", v));
             }
         }"#,
     );

tests/solana_tests/vector_to_slice.rs

@@ -11,7 +11,7 @@ fn test_slice_in_phi() {
             string bst = "from Solang";
 
             while (ast == bst) {
-                ast = ast + "a";
+                ast = string.concat(ast, "a");
             }
 
             return ast;