Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix[codegen]: order of eval for some builtins #4156

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions tests/functional/builtins/codegen/test_extract32.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import pytest

from vyper.evm.opcodes import version_check
from vyper.exceptions import CompilerPanic


@pytest.mark.parametrize("location", ["storage", "transient"])
Expand Down Expand Up @@ -101,8 +100,6 @@ def foq(inp: Bytes[32]) -> address:
c.foq(b"crow" * 8)


# to fix in future release
@pytest.mark.xfail(raises=CompilerPanic, reason="risky overlap")
def test_extract32_order_of_eval(get_contract):
extract32_code = """
var:DynArray[Bytes[96], 1]
Expand All @@ -123,8 +120,6 @@ def foo() -> bytes32:
assert c.foo() == b"defghijklmnopqrstuvwxyz123456789"


# to fix in future release
@pytest.mark.xfail(raises=CompilerPanic, reason="risky overlap")
def test_extract32_order_of_eval_extcall(get_contract):
slice_code = """
var:DynArray[Bytes[96], 1]
Expand Down
6 changes: 1 addition & 5 deletions tests/functional/builtins/codegen/test_slice.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from vyper.compiler import compile_code
from vyper.compiler.settings import OptimizationLevel, Settings
from vyper.evm.opcodes import version_check
from vyper.exceptions import ArgumentException, CompilerPanic, TypeMismatch
from vyper.exceptions import ArgumentException, TypeMismatch

_fun_bytes32_bounds = [(0, 32), (3, 29), (27, 5), (0, 5), (5, 3), (30, 2)]

Expand Down Expand Up @@ -564,8 +564,6 @@ def foo(cs: String[64]) -> uint256:
assert c.foo(arg) == 1


# to fix in future release
@pytest.mark.xfail(raises=CompilerPanic, reason="risky overlap")
def test_slice_order_of_eval(get_contract):
slice_code = """
var:DynArray[Bytes[96], 1]
Expand All @@ -589,8 +587,6 @@ def foo() -> Bytes[96]:
assert c.foo() == b"defghijklmnopqrstuvwxyz123456789"


# to fix in future release
@pytest.mark.xfail(raises=CompilerPanic, reason="risky overlap")
def test_slice_order_of_eval2(get_contract):
slice_code = """
var:DynArray[Bytes[96], 1]
Expand Down
8 changes: 4 additions & 4 deletions vyper/builtins/functions.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
clamp_basetype,
clamp_nonzero,
copy_bytes,
create_memory_copy,
dummy_node_for_type,
ensure_eval_once,
ensure_in_memory,
Expand Down Expand Up @@ -357,9 +358,8 @@ def build_IR(self, expr, args, kwargs, context):
# copy_bytes works on pointers.
assert is_bytes32, src
src = ensure_in_memory(src, context)

if potential_overlap(src, start) or potential_overlap(src, length):
raise CompilerPanic("risky overlap")
elif potential_overlap(src, start) or potential_overlap(src, length):
src = create_memory_copy(src, context)

with src.cache_when_complex("src") as (b1, src), start.cache_when_complex("start") as (
b2,
Expand Down Expand Up @@ -867,7 +867,7 @@ def build_IR(self, expr, args, kwargs, context):
ret_type = kwargs["output_type"]

if potential_overlap(bytez, index):
raise CompilerPanic("risky overlap")
bytez = create_memory_copy(bytez, context)

def finalize(ret):
annotation = "extract32"
Expand Down
8 changes: 8 additions & 0 deletions vyper/codegen/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -1103,6 +1103,14 @@ def ensure_in_memory(ir_var, context):
return IRnode.from_list(["seq", do_copy, buf], typ=typ, location=MEMORY)


# TODO: refactor ensure_in_memory to use this function
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we do this within the scope of this PR?

def create_memory_copy(ir_var, context):
typ = ir_var.typ
buf = context.new_internal_variable(typ)
do_copy = make_setter(buf, ir_var)
return IRnode.from_list(["seq", do_copy, buf], typ=typ, location=MEMORY)


def eval_seq(ir_node):
"""Tries to find the "return" value of a `seq` statement, in order so
that the value can be known without possibly evaluating side effects
Expand Down
Loading