From 2df916ca453ddda4cf08878c4fdaaa55b963686c Mon Sep 17 00:00:00 2001 From: Charles Cooper Date: Mon, 25 Dec 2023 09:21:38 -0500 Subject: [PATCH] feat: improve panics in IR generation (#3708) * feat: improve panics in IR generation this QOL commit improves on 91659266c55a by passing through the `__traceback__` field when the exception is modified (instead of using `__cause__` - cf. PEP-3134 regarding the difference) and improves error messages when an IRnode is not returned properly. using `__traceback__` generally results in a better experience because the immediate cause of the exception is displayed when running `vyper -v` instead of needing to scroll up through the exception chain (if the exception chain is reproduced correctly at all in the first place). --------- Co-authored-by: Harry Kalogirou --- vyper/codegen/expr.py | 30 ++++++++++++++---------------- vyper/codegen/stmt.py | 14 ++++++-------- vyper/exceptions.py | 13 ++++++++----- 3 files changed, 28 insertions(+), 29 deletions(-) diff --git a/vyper/codegen/expr.py b/vyper/codegen/expr.py index 693d5c2aad..4c7c3afaed 100644 --- a/vyper/codegen/expr.py +++ b/vyper/codegen/expr.py @@ -73,19 +73,17 @@ def __init__(self, node, context): self.context = context if isinstance(node, IRnode): - # TODO this seems bad + # this is a kludge for parse_AugAssign to pass in IRnodes + # directly. + # TODO fixme! self.ir_node = node return - fn = getattr(self, f"parse_{type(node).__name__}", None) - if fn is None: - raise TypeCheckFailure(f"Invalid statement node: {type(node).__name__}", node) - - with tag_exceptions(node, fallback_exception_type=CodegenPanic): + fn_name = f"parse_{type(node).__name__}" + with tag_exceptions(node, fallback_exception_type=CodegenPanic, note=fn_name): + fn = getattr(self, fn_name) self.ir_node = fn() - - if self.ir_node is None: - raise TypeCheckFailure(f"{type(node).__name__} node did not produce IR.\n", node) + assert isinstance(self.ir_node, IRnode), self.ir_node self.ir_node.annotation = self.expr.get("node_source_code") self.ir_node.source_pos = getpos(self.expr) @@ -362,9 +360,9 @@ def parse_Subscript(self): index = self.expr.slice.value.n # note: this check should also happen in get_element_ptr if not 0 <= index < len(sub.typ.member_types): - return + raise TypeCheckFailure("unreachable") else: - return + raise TypeCheckFailure("unreachable") ir_node = get_element_ptr(sub, index) ir_node.mutable = sub.mutable @@ -399,13 +397,13 @@ def parse_BinOp(self): new_typ = left.typ if new_typ.bits != 256: # TODO implement me. ["and", 2**bits - 1, shl(right, left)] - return + raise TypeCheckFailure("unreachable") return IRnode.from_list(shl(right, left), typ=new_typ) if isinstance(self.expr.op, vy_ast.RShift): new_typ = left.typ if new_typ.bits != 256: # TODO implement me. promote_signed_int(op(right, left), bits) - return + raise TypeCheckFailure("unreachable") op = shr if not left.typ.is_signed else sar return IRnode.from_list(op(right, left), typ=new_typ) @@ -448,7 +446,7 @@ def build_in_comparator(self): elif isinstance(self.expr.op, vy_ast.NotIn): found, not_found = 0, 1 else: # pragma: no cover - return + raise TypeCheckFailure("unreachable") i = IRnode.from_list(self.context.fresh_varname("in_ix"), typ=UINT256_T) @@ -510,7 +508,7 @@ def parse_Compare(self): right = Expr.parse_value_expr(self.expr.right, self.context) if right.value is None: - return + raise TypeCheckFailure("unreachable") if isinstance(self.expr.op, (vy_ast.In, vy_ast.NotIn)): if is_array_like(right.typ): @@ -562,7 +560,7 @@ def parse_Compare(self): elif left.typ._is_prim_word and right.typ._is_prim_word: if op not in ("eq", "ne"): - return + raise TypeCheckFailure("unreachable") else: # kludge to block behavior in #2638 # TODO actually implement equality for complex types diff --git a/vyper/codegen/stmt.py b/vyper/codegen/stmt.py index 18e5c3d494..bc29a79734 100644 --- a/vyper/codegen/stmt.py +++ b/vyper/codegen/stmt.py @@ -40,16 +40,14 @@ class Stmt: def __init__(self, node: vy_ast.VyperNode, context: Context) -> None: self.stmt = node self.context = context - fn = getattr(self, f"parse_{type(node).__name__}", None) - if fn is None: - raise TypeCheckFailure(f"Invalid statement node: {type(node).__name__}") - with context.internal_memory_scope(): - with tag_exceptions(node, fallback_exception_type=CodegenPanic): + fn_name = f"parse_{type(node).__name__}" + with tag_exceptions(node, fallback_exception_type=CodegenPanic, note=fn_name): + fn = getattr(self, fn_name) + with context.internal_memory_scope(): self.ir_node = fn() - if self.ir_node is None: - raise TypeCheckFailure("Statement node did not produce IR") + assert isinstance(self.ir_node, IRnode), self.ir_node self.ir_node.annotation = self.stmt.get("node_source_code") self.ir_node.source_pos = getpos(self.stmt) @@ -347,7 +345,7 @@ def parse_AugAssign(self): # because of this check, we do not need to check for # make_setter references lhs<->rhs as in parse_Assign - # single word load/stores are atomic. - return + raise TypeCheckFailure("unreachable") with target.cache_when_complex("_loc") as (b, target): rhs = Expr.parse_value_expr( diff --git a/vyper/exceptions.py b/vyper/exceptions.py index 8921814188..f216069eab 100644 --- a/vyper/exceptions.py +++ b/vyper/exceptions.py @@ -359,14 +359,17 @@ class InvalidABIType(VyperInternalException): @contextlib.contextmanager -def tag_exceptions( - node, fallback_exception_type=CompilerPanic, fallback_message="unhandled exception" -): +def tag_exceptions(node, fallback_exception_type=CompilerPanic, note=None): try: yield except _BaseVyperException as e: if not e.annotations and not e.lineno: - raise e.with_annotation(node) from None + tb = e.__traceback__ + raise e.with_annotation(node).with_traceback(tb) raise e from None except Exception as e: - raise fallback_exception_type(fallback_message, node) from e + tb = e.__traceback__ + fallback_message = "unhandled exception" + if note: + fallback_message += f", {note}" + raise fallback_exception_type(fallback_message, node).with_traceback(tb)