fix[codegen]: fix removal of side effects in concat (#4644)

concat would remove side effects for zero-length arguments. fix by
removing the fastpath.

as the test case shows, this pattern is not common in user-code.

this commit also reverts the optimization introduced in a06f7df,
and replaces it with a tag on the `IRnode`, since otherwise the
`~empty` intrinsic can show up in the IR at the time we lower to
assembly (which is an error).

---------

Co-authored-by: cyberthirst <[email protected]>

Author: charles-cooper

tests/functional/builtins/codegen/test_concat.py

@@ -170,6 +170,48 @@ def hoo(x: bytes32, y: bytes32) -> Bytes[64]:
     print("Passed second concat tests")
 
 
+def test_concat_zero_length_side_effects(get_contract):
+    code = """
+counter: public(uint256)
+
+@external
+def test() -> Bytes[256]:
+    a: Bytes[256] = concat(b"" if self.sideeffect() else b"", b"aaaa")
+    return a
+
+def sideeffect() -> bool:
+    self.counter += 1
+    return True
+    """
+
+    c = get_contract(code)
+
+    assert c.counter() == 0
+    assert c.test() == b"aaaa"
+    assert c.counter() == 1
+
+
+def test_concat_zero_length_side_effects2(get_contract):
+    code = """
+counter: public(uint256)
+
+@external
+def test() -> Bytes[256]:
+    a: Bytes[256] = concat(b"" if self.sideeffect() else b"", b"")
+    return a
+
+def sideeffect() -> bool:
+    self.counter += 1
+    return True
+    """
+
+    c = get_contract(code)
+
+    assert c.counter() == 0
+    assert c.test() == b""
+    assert c.counter() == 1
+
+
 def test_small_output(get_contract):
     code = """
 @external

vyper/builtins/functions.py

@@ -561,10 +561,6 @@ def build_IR(self, expr, context):
             dst_data = add_ofst(bytes_data_ptr(dst), ofst)
 
             if isinstance(arg.typ, _BytestringT):
-                # Ignore empty strings
-                if arg.typ.maxlen == 0:
-                    continue
-
                 with arg.cache_when_complex("arg") as (b1, arg):
                     argdata = bytes_data_ptr(arg)
 

vyper/codegen/expr.py

@@ -147,10 +147,6 @@ def _make_bytelike(cls, context, typeclass, bytez):
         bytez_length = len(bytez)
         btype = typeclass(bytez_length)
 
-        if bytez_length == 0:
-            # optimization: handled specially by make_byte_array_copier
-            return IRnode.from_list("~empty", typ=btype, annotation=f"empty {btype}")
-
         placeholder = context.new_internal_variable(btype)
         seq = []
         seq.append(["mstore", placeholder, bytez_length])
@@ -163,12 +159,14 @@ def _make_bytelike(cls, context, typeclass, bytez):
                 ]
             )
 
-        return IRnode.from_list(
+        ret = IRnode.from_list(
             ["seq"] + seq + [placeholder],
             typ=btype,
             location=MEMORY,
             annotation=f"Create {btype}: {bytez}",
         )
+        ret.is_source_bytes_literal = True
+        return ret
 
     # True, False, None constants
     def parse_NameConstant(self):

vyper/codegen/ir_node.py

@@ -10,7 +10,7 @@
 from vyper.evm.address_space import AddrSpace
 from vyper.evm.opcodes import get_ir_opcodes
 from vyper.exceptions import CodegenPanic, CompilerPanic
-from vyper.semantics.types import VyperType
+from vyper.semantics.types import VyperType, _BytestringT
 from vyper.utils import VALID_IR_MACROS, ceil32
 
 # Set default string representation for ints in IR output.
@@ -139,6 +139,10 @@ class IRnode:
     func_ir: Any
     common_ir: Any
 
+    # for bytestrings, if we have `is_source_bytes_literal`, we can perform
+    # certain optimizations like eliding the copy.
+    is_source_bytes_literal: bool = False
+
     def __init__(
         self,
         value: Union[str, int],
@@ -367,6 +371,13 @@ def gas(self):
     def is_empty_intrinsic(self):
         if self.value == "~empty":
             return True
+        if (
+            self.is_source_bytes_literal
+            and isinstance(self.typ, _BytestringT)
+            and self.typ.maxlen == 0
+        ):
+            # special optimization case for empty `b""` literal
+            return True
         if self.value == "seq":
             return len(self.args) == 1 and self.args[0].is_empty_intrinsic
         return False