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.

Sign up for GitHub

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: raise UNREACHABLE #3194

Merged
merged 12 commits into from
Jan 16, 2023
Merged

fix: raise UNREACHABLE #3194

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
af12adf
wip: fix raise UNREACHABLE issue #3070
emc415 Dec 15, 2022
3164502
simplified parse_Raise: pass 0 instead of None as condition to _asser…
emc415 Dec 19, 2022
882c35e
Added handling to vyper/ir/compile_ir.py to handle raise_unreachable
emc415 Dec 23, 2022
51dce27
Merge branch 'vyperlang:master' into fix/raise_unreachable
emc415 Dec 24, 2022
3ff7cf4
changed codegen/stmt.py to emit "invalid" instead of "raise_unreachable"
emc415 Dec 24, 2022
1cf6f2f
moved raise UNREACHABLE handling to _assert_reason so that all UNREAC…
emc415 Dec 26, 2022
7593b03
replacing vestigial "assure" in the codebase with current "unreachable"
emc415 Dec 26, 2022
66e57e8
changed the logic in _assert_reason so that AST is no longer being mo…
emc415 Dec 28, 2022
428789f
added variable is_raise to factor out test_expr is None
emc415 Dec 29, 2022
c12c372
formatted comment with linter
emc415 Dec 29, 2022
570792f
Added test_raise_unreachable
emc415 Dec 29, 2022
d4a14af
remove redundant assert
charles-cooper Jan 16, 2023
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
2 changes: 1 addition & 1 deletion tests/parser/features/test_assert_unreachable.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
def test_assure_refund(w3, get_contract):
def test_unreachable_refund(w3, get_contract):
code = """
@external
def foo():
Expand Down
12 changes: 6 additions & 6 deletions vyper/codegen/stmt.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,9 +160,12 @@ def parse_Call(self):

def _assert_reason(self, test_expr, msg):
if isinstance(msg, vy_ast.Name) and msg.id == "UNREACHABLE":
return IRnode.from_list(
["assert_unreachable", test_expr], error_msg="assert unreachable"
)
if test_expr is not None:
return IRnode.from_list(
["assert_unreachable", test_expr], error_msg="assert unreachable"
)
else:
return IRnode.from_list(["invalid"], error_msg="raise unreachable")
Copy link
Member

Choose a reason for hiding this comment

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

nice -- since this check is performed twice, and it's actually not entirely clear what the check is for, i would factor out the expression (test_expr is None) into a variable called is_raise and add a comment about what it is doing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cool! Is it possible that None can be passed into parse_Assert from an internal compiler assert? I am just thinking that if so, it may be a little misleading if it got called 'is_raise', but also it shouldn't affect behavior.

Copy link
Member

Choose a reason for hiding this comment

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

no, parse_Assert only handles user asserts -- but if you are paranoid (which is not a bad thing!) you can add a sanity check in parse_Assert on the test expr before passing down into assert_reason().


# set constant so that revert reason str is well behaved
try:
Expand Down Expand Up @@ -205,17 +208,14 @@ def _get_last(ir):
["mstore", buf - 32, 0x20],
["revert", buf - 36, ["add", 4 + 32 + 32, ["ceil32", _runtime_length]]],
]

if test_expr is not None:
ir_node = ["if", ["iszero", test_expr], revert_seq]
else:
ir_node = revert_seq

return IRnode.from_list(ir_node, error_msg="user revert with reason")

def parse_Assert(self):
test_expr = Expr.parse_value_expr(self.stmt.test, self.context)

if self.stmt.msg:
return self._assert_reason(test_expr, self.stmt.msg)
else:
Expand Down
2 changes: 1 addition & 1 deletion vyper/ir/compile_ir.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -537,7 +537,7 @@ def _height_of(witharg):
o.append("POP")
return o
# Seq without popping.
# Assure (if false, invalid opcode)
# unreachable keyword produces INVALID opcode
elif code.value == "assert_unreachable":
o = _compile_to_assembly(code.args[0], withargs, existing_labels, break_dest, height)
end_symbol = mksymbol("reachable")
Expand Down