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

[seanyoung]

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

Fixes: #1558

[codecov]

Codecov Report

Merging #1590 (5a701d9) into main (e4b9a47) will increase coverage by 0.08%.
The diff coverage is 99.23%.

@@            Coverage Diff             @@
##             main    #1590      +/-   ##
==========================================
+ Coverage   87.57%   87.65%   +0.08%     
==========================================
  Files         133      133              
  Lines       64217    64300      +83     
==========================================
+ Hits        56236    56363     +127     
+ Misses       7981     7937      -44     

Files	Coverage Δ
src/bin/languageserver/mod.rs	79.33% <ø> (+0.29%)	⬆️
src/codegen/cfg.rs	93.94% <ø> (-0.02%)	⬇️
src/codegen/events/polkadot.rs	100.00% <100.00%> (ø)
src/codegen/expression.rs	95.24% <ø> (-0.02%)	⬇️
src/codegen/mod.rs	83.72% <100.00%> (-0.16%)	⬇️
...expression_elimination/available_expression_set.rs	99.75% <100.00%> (-0.01%)	⬇️
...rc/codegen/subexpression_elimination/expression.rs	98.15% <ø> (+0.60%)	⬆️
src/codegen/subexpression_elimination/operator.rs	96.29% <ø> (-0.07%)	⬇️
src/emit/expression.rs	98.88% <100.00%> (+0.03%)	⬆️
src/sema/ast.rs	87.07% <100.00%> (-0.04%)	⬇️
... and 6 more

[LucasSte]

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

Fixes: #1558

Removing a+b is a breaking change and renders existing contracts incompatible. Shouldn't we first warn that a+b is deprecated and then remove it in a later release?

[seanyoung]

Removing a+b is a breaking change and renders existing contracts incompatible. Shouldn't we first warn that a+b is deprecated and then remove it in a later release?

a+b for strings was always a non-standard extension. It is not used much; for example, in the solana program examples it is not used (I've also just tested that).

As such I don't think there is need for deprecation and eventual removal. I've added an entry to CHANGELOG.md.

[xermicus]

LGTM, I just see some potential to optimize the concatenation into a no-op when one operand is an empty constant:

function f(string a) public returns (string) {
    return string.concat(a, "");
}

block0: # entry
        ty:string %a = (arg #0)
        return (builtin Concat ((arg #0), (alloc string uint32 0 "")))

[seanyoung]

LGTM, I just see some potential to optimize the concatenation into a no-op when one operand is an empty constant:

function f(string a) public returns (string) {
    return string.concat(a, "");
}

block0: # entry
        ty:string %a = (arg #0)
        return (builtin Concat ((arg #0), (alloc string uint32 0 "")))

Nice, good catch. I've fixed this and added a test case.

[LucasSte]

Removing a+b is a breaking change and renders existing contracts incompatible. Shouldn't we first warn that a+b is deprecated and then remove it in a later release?

It is not used much;

We don't know if it is not used much. There is no way to prove it. The solana examples is a small set of what people use Solang for. Polkadot also support this feature.

Instead of deprecating then, what do you think of suggesting people to use string.concat when erroring for string + string?

[seanyoung]

Instead of deprecating then, what do you think of suggesting people to use string.concat when erroring for string + string?

I've added a diagnostic for this. I think it's useful in general for folks when they may try to concatenate two strings.