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

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

Fixes: hyperledger#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)
         }