[mypyc] Do not emit tracebacks with negative line numbers (#20641)

mypyc may add tracebacks in the generated code with line number -1 when
the AST expression that generates an error has no line set. This is
usually the case for expressions that are not directly linked with a
line in the Python code but are generated internally by mypyc, like
helper functions generated for `async` functions.

These tracebacks become a problem when running a test that calls a
compiled function with pytest. The line number ends up being interpreted
by Python as `None` instead of an `int` and pytest crashes when [trying
to do math on
it](https://github.com/pytest-dev/pytest/blob/main/src/_pytest/_code/code.py#L211).
So instead of a stack trace pointing to the compiled function we get an
internal pytest error which makes debugging more difficult.

Changes in this PR ensure that AST nodes that may produce tracebacks
have their line numbers set. For AST nodes generated by mypyc this
usually means the line number of the function they are associated with,
so an internal attribute access node inside a helper function generated
for an `async def my_fun()` will have the line number of `my_fun`.

I didn't add a specific test for this because getting an exception from
an internally generated node means that there is a bug in mypyc that we
should fix but I tested it with one such error that I'm working on and
pytest no longer crashes. I have also added assertions so that new
changes to mypyc don't cause it to generate tracebacks with negative
line numbers.

For now the assertions are only enabled in tests since there are cases
where mypyc emits tracebacks that we don't have tests for, and the new
assertions could break compilation that currently succeeds without them.
Once we remove all cases where tracebacks with negative line numbers are
possible we can enable them by default.

Author: p-sawicki

mypyc/codegen/emit.py

@@ -103,6 +103,7 @@ class EmitterContext:
     def __init__(
         self,
         names: NameGenerator,
+        strict_traceback_checks: bool,
         group_name: str | None = None,
         group_map: dict[str, str | None] | None = None,
     ) -> None:
@@ -130,6 +131,8 @@ def __init__(
         self.declarations: dict[str, HeaderDeclaration] = {}
 
         self.literals = Literals()
+        # See mypyc/options.py for context.
+        self.strict_traceback_checks = strict_traceback_checks
 
 
 class ErrorHandler:
@@ -1201,6 +1204,8 @@ def _emit_traceback(
         type_str: str = "",
         src: str = "",
     ) -> None:
+        if self.context.strict_traceback_checks:
+            assert traceback_entry[1] >= 0, "Traceback cannot have a negative line number"
         globals_static = self.static_name("globals", module_name)
         line = '%s("%s", "%s", %d, %s' % (
             func,

mypyc/codegen/emitfunc.py

@@ -921,6 +921,10 @@ def emit_traceback(self, op: Branch) -> None:
 
     def emit_attribute_error(self, op: Branch, class_name: str, attr: str) -> None:
         assert op.traceback_entry is not None
+        if self.emitter.context.strict_traceback_checks:
+            assert (
+                op.traceback_entry[1] >= 0
+            ), "AttributeError traceback cannot have a negative line number"
         globals_static = self.emitter.static_name("globals", self.module_name)
         self.emit_line(
             'CPy_AttributeError("%s", "%s", "%s", "%s", %d, %s);'

mypyc/codegen/emitmodule.py

@@ -249,9 +249,9 @@ def compile_scc_to_ir(
     for module in modules.values():
         for fn in module.functions:
             # Insert checks for uninitialized values.
-            insert_uninit_checks(fn)
+            insert_uninit_checks(fn, compiler_options.strict_traceback_checks)
             # Insert exception handling.
-            insert_exception_handling(fn)
+            insert_exception_handling(fn, compiler_options.strict_traceback_checks)
             # Insert reference count handling.
             insert_ref_count_opcodes(fn)
 
@@ -535,7 +535,9 @@ def __init__(
         """
         self.modules = modules
         self.source_paths = source_paths
-        self.context = EmitterContext(names, group_name, group_map)
+        self.context = EmitterContext(
+            names, compiler_options.strict_traceback_checks, group_name, group_map
+        )
         self.names = names
         # Initializations of globals to simple values that we can't
         # do statically because the windows loader is bad.

mypyc/ir/ops.py

@@ -773,9 +773,10 @@ class PrimitiveOp(RegisterOp):
     """
 
     def __init__(self, args: list[Value], desc: PrimitiveDescription, line: int = -1) -> None:
+        self.error_kind = desc.error_kind
+        super().__init__(line)
         self.args = args
         self.type = desc.return_type
-        self.error_kind = desc.error_kind
         self.desc = desc
 
     def sources(self) -> list[Value]:
@@ -849,7 +850,8 @@ class LoadLiteral(RegisterOp):
     error_kind = ERR_NEVER
     is_borrowed = True
 
-    def __init__(self, value: LiteralValue, rtype: RType) -> None:
+    def __init__(self, value: LiteralValue, rtype: RType, line: int = -1) -> None:
+        super().__init__(line)
         self.value = value
         self.type = rtype
 
@@ -1800,7 +1802,8 @@ class KeepAlive(RegisterOp):
 
     error_kind = ERR_NEVER
 
-    def __init__(self, src: list[Value], *, steal: bool = False) -> None:
+    def __init__(self, src: list[Value], line: int = -1, *, steal: bool = False) -> None:
+        super().__init__(line)
         assert src
         self.src = src
         self.steal = steal

mypyc/irbuild/ast_helpers.py

@@ -90,12 +90,12 @@ def maybe_process_conditional_comparison(
         elif not is_fixed_width_rtype(rtype):
             right = self.coerce(right, ltype, e.line)
         reg = self.binary_op(left, right, op, e.line)
-        self.builder.flush_keep_alives()
+        self.builder.flush_keep_alives(e.line)
         self.add_bool_branch(reg, true, false)
     else:
         # "left op right" for two tagged integers
         reg = self.builder.binary_op(left, right, op, e.line)
-        self.flush_keep_alives()
+        self.flush_keep_alives(e.line)
         self.add_bool_branch(reg, true, false)
     return True
 

mypyc/irbuild/builder.py

@@ -66,6 +66,7 @@
 from mypyc.ir.ops import (
     NAMESPACE_MODULE,
     NAMESPACE_TYPE_VAR,
+    NO_TRACEBACK_LINE_NO,
     Assign,
     BasicBlock,
     Branch,
@@ -288,7 +289,7 @@ def accept(self, node: Statement | Expression, *, can_borrow: bool = False) -> V
                     res = Register(self.node_type(node))
                 self.can_borrow = old_can_borrow
                 if not can_borrow:
-                    self.flush_keep_alives()
+                    self.flush_keep_alives(node.line)
                 return res
             else:
                 try:
@@ -297,8 +298,8 @@ def accept(self, node: Statement | Expression, *, can_borrow: bool = False) -> V
                     pass
                 return None
 
-    def flush_keep_alives(self) -> None:
-        self.builder.flush_keep_alives()
+    def flush_keep_alives(self, line: int) -> None:
+        self.builder.flush_keep_alives(line)
 
     # Pass through methods for the most common low-level builder ops, for convenience.
 
@@ -320,23 +321,23 @@ def self(self) -> Register:
     def py_get_attr(self, obj: Value, attr: str, line: int) -> Value:
         return self.builder.py_get_attr(obj, attr, line)
 
-    def load_str(self, value: str) -> Value:
-        return self.builder.load_str(value)
+    def load_str(self, value: str, line: int = -1) -> Value:
+        return self.builder.load_str(value, line)
 
-    def load_bytes_from_str_literal(self, value: str) -> Value:
+    def load_bytes_from_str_literal(self, value: str, line: int = -1) -> Value:
         """Load bytes object from a string literal.
 
         The literal characters of BytesExpr (the characters inside b'')
         are stored in BytesExpr.value, whose type is 'str' not 'bytes'.
         Thus we perform a special conversion here.
         """
-        return self.builder.load_bytes(bytes_from_str(value))
+        return self.builder.load_bytes(bytes_from_str(value), line)
 
-    def load_int(self, value: int) -> Value:
-        return self.builder.load_int(value)
+    def load_int(self, value: int, line: int = -1) -> Value:
+        return self.builder.load_int(value, line)
 
-    def load_float(self, value: float) -> Value:
-        return self.builder.load_float(value)
+    def load_float(self, value: float, line: int = -1) -> Value:
+        return self.builder.load_float(value, line)
 
     def unary_op(self, lreg: Value, expr_op: str, line: int) -> Value:
         return self.builder.unary_op(lreg, expr_op, line)
@@ -347,17 +348,17 @@ def binary_op(self, lreg: Value, rreg: Value, expr_op: str, line: int) -> Value:
     def coerce(self, src: Value, target_type: RType, line: int, force: bool = False) -> Value:
         return self.builder.coerce(src, target_type, line, force, can_borrow=self.can_borrow)
 
-    def none_object(self) -> Value:
-        return self.builder.none_object()
+    def none_object(self, line: int = -1) -> Value:
+        return self.builder.none_object(line)
 
-    def none(self) -> Value:
-        return self.builder.none()
+    def none(self, line: int = -1) -> Value:
+        return self.builder.none(line)
 
-    def true(self) -> Value:
-        return self.builder.true()
+    def true(self, line: int = -1) -> Value:
+        return self.builder.true(line)
 
-    def false(self) -> Value:
-        return self.builder.false()
+    def false(self, line: int = -1) -> Value:
+        return self.builder.false(line)
 
     def new_list_op(self, values: list[Value], line: int) -> Value:
         return self.builder.new_list_op(values, line)
@@ -438,7 +439,7 @@ def add_to_non_ext_dict(
         self, non_ext: NonExtClassInfo, key: str, val: Value, line: int
     ) -> None:
         # Add an attribute entry into the class dict of a non-extension class.
-        key_unicode = self.load_str(key)
+        key_unicode = self.load_str(key, line)
         self.primitive_op(dict_set_item_op, [non_ext.dict, key_unicode, val], line)
 
         # It's important that accessing class dictionary items from multiple threads
@@ -452,7 +453,7 @@ def gen_import(self, id: str, line: int) -> None:
         self.check_if_module_loaded(id, line, needs_import, out)
 
         self.activate_block(needs_import)
-        value = self.call_c(import_op, [self.load_str(id)], line)
+        value = self.call_c(import_op, [self.load_str(id, line)], line)
         self.add(InitStatic(value, id, namespace=NAMESPACE_MODULE))
         self.goto_and_activate(out)
 
@@ -467,14 +468,14 @@ def check_if_module_loaded(
             needs_import: the BasicBlock that is run if the module has not been loaded yet
             out: the BasicBlock that is run if the module has already been loaded"""
         first_load = self.load_module(id)
-        comparison = self.translate_is_op(first_load, self.none_object(), "is not", line)
+        comparison = self.translate_is_op(first_load, self.none_object(line), "is not", line)
         self.add_bool_branch(comparison, out, needs_import)
 
     def get_module(self, module: str, line: int) -> Value:
         # Python 3.7 has a nice 'PyImport_GetModule' function that we can't use :(
         mod_dict = self.call_c(get_module_dict_op, [], line)
         # Get module object from modules dict.
-        return self.primitive_op(dict_get_item_op, [mod_dict, self.load_str(module)], line)
+        return self.primitive_op(dict_get_item_op, [mod_dict, self.load_str(module, line)], line)
 
     def get_module_attr(self, module: str, attr: str, line: int) -> Value:
         """Look up an attribute of a module without storing it in the local namespace.
@@ -491,9 +492,9 @@ def get_module_attr(self, module: str, attr: str, line: int) -> Value:
     def assign_if_null(self, target: Register, get_val: Callable[[], Value], line: int) -> None:
         """If target is NULL, assign value produced by get_val to it."""
         error_block, body_block = BasicBlock(), BasicBlock()
-        self.add(Branch(target, error_block, body_block, Branch.IS_ERROR))
+        self.add(Branch(target, error_block, body_block, Branch.IS_ERROR, line))
         self.activate_block(error_block)
-        self.add(Assign(target, self.coerce(get_val(), target.type, line)))
+        self.add(Assign(target, self.coerce(get_val(), target.type, line), line))
         self.goto(body_block)
         self.activate_block(body_block)
 
@@ -508,7 +509,7 @@ def assign_if_bitmap_unset(
             IntOp.AND,
             line,
         )
-        b = self.add(ComparisonOp(o, Integer(0, bitmap_rprimitive), ComparisonOp.EQ))
+        b = self.add(ComparisonOp(o, Integer(0, bitmap_rprimitive), ComparisonOp.EQ, line))
         self.add(Branch(b, error_block, body_block, Branch.BOOL))
         self.activate_block(error_block)
         self.add(Assign(target, self.coerce(get_val(), target.type, line)))
@@ -524,8 +525,9 @@ def maybe_add_implicit_return(self) -> None:
     def add_implicit_return(self) -> None:
         block = self.builder.blocks[-1]
         if not block.terminated:
-            retval = self.coerce(self.builder.none(), self.ret_types[-1], -1)
-            self.nonlocal_control[-1].gen_return(self, retval, self.fn_info.fitem.line)
+            line = self.fn_info.fitem.line
+            retval = self.coerce(self.builder.none(), self.ret_types[-1], line)
+            self.nonlocal_control[-1].gen_return(self, retval, line)
 
     def add_implicit_unreachable(self) -> None:
         block = self.builder.blocks[-1]
@@ -605,13 +607,15 @@ def load_type_var(self, name: str, line: int) -> Value:
             )
         )
 
-    def load_literal_value(self, val: int | str | bytes | float | complex | bool) -> Value:
+    def load_literal_value(
+        self, val: int | str | bytes | float | complex | bool, line: int = -1
+    ) -> Value:
         """Load value of a final name, class-level attribute, or constant folded expression."""
         if isinstance(val, bool):
             if val:
-                return self.true()
+                return self.true(line)
             else:
-                return self.false()
+                return self.false(line)
         elif isinstance(val, int):
             return self.builder.load_int(val)
         elif isinstance(val, float):
@@ -669,7 +673,7 @@ def get_assignment_target(
                     return self.lookup(symbol)
             elif lvalue.kind == GDEF:
                 globals_dict = self.load_globals_dict()
-                name = self.load_str(lvalue.name)
+                name = self.load_str(lvalue.name, line)
                 return AssignmentTargetIndex(globals_dict, name)
             else:
                 assert False, lvalue.kind
@@ -745,23 +749,23 @@ def read_nullable_attr(self, obj: Value, attr: str, line: int = -1) -> Value:
 
     def assign(self, target: Register | AssignmentTarget, rvalue_reg: Value, line: int) -> None:
         if isinstance(target, Register):
-            self.add(Assign(target, self.coerce_rvalue(rvalue_reg, target.type, line)))
+            self.add(Assign(target, self.coerce_rvalue(rvalue_reg, target.type, line), line))
         elif isinstance(target, AssignmentTargetRegister):
             rvalue_reg = self.coerce_rvalue(rvalue_reg, target.type, line)
-            self.add(Assign(target.register, rvalue_reg))
+            self.add(Assign(target.register, rvalue_reg, line))
         elif isinstance(target, AssignmentTargetAttr):
             if isinstance(target.obj_type, RInstance):
                 setattr = target.obj_type.class_ir.get_method("__setattr__")
                 if setattr:
-                    key = self.load_str(target.attr)
+                    key = self.load_str(target.attr, line)
                     boxed_reg = self.builder.box(rvalue_reg)
                     call = MethodCall(target.obj, setattr.name, [key, boxed_reg], line)
                     self.add(call)
                 else:
                     rvalue_reg = self.coerce_rvalue(rvalue_reg, target.type, line)
                     self.add(SetAttr(target.obj, target.attr, rvalue_reg, line))
             else:
-                key = self.load_str(target.attr)
+                key = self.load_str(target.attr, line)
                 boxed_reg = self.builder.box(rvalue_reg)
                 self.primitive_op(py_setattr_op, [target.obj, key, boxed_reg], line)
         elif isinstance(target, AssignmentTargetIndex):
@@ -931,8 +935,8 @@ def make_spill_target(self, type: RType) -> AssignmentTarget:
     def spill(self, value: Value) -> AssignmentTarget:
         """Moves a given Value instance into the generator class' environment class."""
         target = self.make_spill_target(value.type)
-        # Shouldn't be able to fail, so -1 for line
-        self.assign(target, value, -1)
+        # Shouldn't be able to fail
+        self.assign(target, value, NO_TRACEBACK_LINE_NO)
         return target
 
     def maybe_spill(self, value: Value) -> Value | AssignmentTarget:
@@ -963,7 +967,7 @@ def maybe_spill_assignable(self, value: Value) -> Register | AssignmentTarget:
 
         # Allocate a temporary register for the assignable value.
         reg = Register(value.type)
-        self.assign(reg, value, -1)
+        self.assign(reg, value, NO_TRACEBACK_LINE_NO)
         return reg
 
     def extract_int(self, e: Expression) -> int | None:
@@ -1132,7 +1136,7 @@ def emit_load_final(
             line: line number where loading occurs
         """
         if final_var.final_value is not None:  # this is safe even for non-native names
-            return self.load_literal_value(final_var.final_value)
+            return self.load_literal_value(final_var.final_value, line)
         elif native and module_prefix(self.graph, fullname):
             return self.load_final_static(fullname, self.mapper.type_to_rtype(typ), line, name)
         else:
@@ -1409,7 +1413,7 @@ def load_global(self, expr: NameExpr) -> Value:
 
     def load_global_str(self, name: str, line: int) -> Value:
         _globals = self.load_globals_dict()
-        reg = self.load_str(name)
+        reg = self.load_str(name, line)
         return self.primitive_op(dict_get_item_op, [_globals, reg], line)
 
     def load_globals_dict(self) -> Value:
@@ -1473,7 +1477,7 @@ def add_coroutine_setup_call(self, class_name: str, obj: Value) -> Value:
                     FuncSignature([RuntimeArg("type", object_rprimitive)], bool_rprimitive),
                 ),
                 [obj],
-                -1,
+                obj.line,
             )
         )
 
@@ -1571,13 +1575,13 @@ def create_type_params(
             # To match runtime semantics, pass infer_variance=True
             tv = builder.py_call(
                 tvt,
-                [builder.load_str(type_param.name), builder.true()],
+                [builder.load_str(type_param.name, line), builder.true(line)],
                 line,
                 arg_kinds=[ARG_POS, ARG_NAMED],
                 arg_names=[None, "infer_variance"],
             )
         else:
-            tv = builder.py_call(tvt, [builder.load_str(type_param.name)], line)
+            tv = builder.py_call(tvt, [builder.load_str(type_param.name, line)], line)
         builder.init_type_var(tv, type_param.name, line)
         tvs.append(tv)
     return tvs