8000 Make refcounting handle error values loaded into registers (#10819) · python/mypy@d0bd1c8 · GitHub
[go: up one dir, main page]
Skip to content

Commit d0bd1c8

Browse files
msullivanmsullivan
authored
Make refcounting handle error values loaded into registers (#10819)
Currently the finally path has a hacky pointless branch on a register that is potentially NULL in order to convince refcounting to do the right thing. I want to add another case that would require this, so just fix refcounting to properly account for error values in registers.
1 parent 7d69ce2 commit d0bd1c8

File tree

4 files changed

+37
-32
lines changed

4 files changed

+37
-32
lines changed

‎mypyc/analysis/dataflow.py

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -236,8 +236,19 @@ class DefinedVisitor(BaseAnalysisVisitor):
236236
Note that this only deals with registers and not temporaries, on
237237
the assumption that we never access temporaries when they might be
238238
undefined.
239+
240+
If strict_errors is True, then we regard any use of LoadErrorValue
241+
as making a register undefined. Otherwise we only do if
242+
`undefines` is set on the error value.
243+
244+
This lets us only consider the things we care about during
245+
uninitialized variable checking while capturing all possibly
246+
undefined things for refcounting.
239247
"""
240248

249+
def __init__(self, strict_errors: bool = False) -> None:
250+
self.strict_errors = strict_errors
251+
241252
def visit_branch(self, op: Branch) -> GenAndKill:
242253
return set(), set()
243254

@@ -252,7 +263,8 @@ def visit_register_op(self, op: RegisterOp) -> GenAndKill:
252263

253264
def visit_assign(self, op: Assign) -> GenAndKill:
254265
# Loading an error value may undefine the register.
255-
if isinstance(op.src, LoadErrorValue) and op.src.undefines:
266+
if (isinstance(op.src, LoadErrorValue)
267+
and (op.src.undefines or self.strict_errors)):
256268
return set(), {op.dest}
257269
else:
258270
return {op.dest}, set()
@@ -284,7 +296,8 @@ def analyze_must_defined_regs(
284296
blocks: List[BasicBlock],
285297
cfg: CFG,
286298
initial_defined: Set[Value],
287-
regs: Iterable[Value]) -> AnalysisResult[Value]:
299+
regs: Iterable[Value],
300+
strict_errors: bool = False) -> AnalysisResult[Value]:
288301
"""Calculate always defined registers at each CFG location.
289302
290303
This analysis can work before exception insertion, since it is a
@@ -296,7 +309,7 @@ def analyze_must_defined_regs(
296309
"""
297310
return run_analysis(blocks=blocks,
298311
cfg=cfg,
299-
gen_and_kill=DefinedVisitor(),
312+
gen_and_kill=DefinedVisitor(strict_errors=strict_errors),
300313
initial=initial_defined,
301314
backward=False,
302315
kind=MUST_ANALYSIS,

‎mypyc/irbuild/nonlocalcontrol.py

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -176,14 +176,6 @@ def __init__(self, outer: NonlocalControl, ret_reg: Optional[Value], saved: Valu
176176
self.saved = saved
177177

178178
def gen_cleanup(self, builder: 'IRBuilder', line: int) -> None:
179-
# Do an error branch on the return value register, which
180-
# may be undefined. This will allow it to be properly
181-
# decrefed if it is not null. This is kind of a hack.
182-
if self.ret_reg:
183-
target = BasicBlock()
184-
builder.add(Branch(self.ret_reg, target, target, Branch.IS_ERROR))
185-
builder.activate_block(target)
186-
187179
# Restore the old exc_info
188180
target, cleanup = BasicBlock(), BasicBlock()
189181
builder.add(Branch(self.saved, target, cleanup, Branch.IS_ERROR))

‎mypyc/test-data/exceptions.test

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,7 @@ L1:
259259
L2:
260260
r3 = PyObject_CallFunctionObjArgs(r2, 0)
261261
dec_ref r2
262-
if is_error(r3) goto L5 (error at a:3) else goto L20
262+
if is_error(r3) goto L5 (error at a:3) else goto L19
263263
L3:
264264
r4 = 'hi'
265265
inc_ref r4
@@ -278,52 +278,52 @@ L6:
278278
r11 = builtins :: module
279279
r12 = 'print'
280280
r13 = CPyObject_GetAttr(r11, r12)
281-
if is_error(r13) goto L13 (error at a:6) else goto L7
281+
if is_error(r13) goto L20 (error at a:6) else goto L7
282282
L7:
283283
r14 = PyObject_CallFunctionObjArgs(r13, r10, 0)
284284
dec_ref r13
285-
if is_error(r14) goto L13 (error at a:6) else goto L21
285+
if is_error(r14) goto L20 (error at a:6) else goto L21
286286
L8:
287-
if is_error(r7) goto L11 else goto L9
287+
if is_error(r7) goto L11 else goto L22
288288
L9:
289289
CPy_Reraise()
290-
if not 0 goto L13 else goto L22 :: bool
290+
if not 0 goto L13 else goto L23 :: bool
291291
L10:
292292
unreachable
293293
L11:
294-
if is_error(r5) goto L18 else goto L12
294+
if is_error(r5) goto L17 else goto L12
295295
L12:
296296
return r5
297297
L13:
298-
if is_error(r5) goto L14 else goto L23
298+
if is_error(r7) goto L15 else goto L14
299299
L14:
300-
if is_error(r7) goto L16 else goto L15
301-
L15:
302300
CPy_RestoreExcInfo(r7)
303-
dec_ref r7
304-
L16:
301+
xdec_ref r7
302+
L15:
305303
r15 = CPy_KeepPropagating()
306-
if not r15 goto L19 else goto L17 :: bool
304+
if not r15 goto L18 else goto L16 :: bool
305+
L16:
306+
unreachable
307307
L17:
308308
unreachable
309309
L18:
310-
unreachable
311-
L19:
312310
r16 = <error> :: str
313311
return r16
314-
L20:
312+
L19:
315313
dec_ref r3
316314
goto L3
315+
L20:
316+
xdec_ref r5
317+
goto L13
317318
L21:
318319
dec_ref r14
319320
goto L8
320321
L22:
321-
dec_ref r5
322-
dec_ref r7
323-
goto L10
322+
xdec_ref r5
323+
goto L9
324324
L23:
325-
dec_ref r5
326-
goto L14
325+
xdec_ref r7
326+
goto L10
327327

328328
[case testDocstring1]
329329
def lol() -> None:

‎mypyc/transform/refcount.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ def insert_ref_count_opcodes(ir: FuncIR) -> None:
5353
args: Set[Value] = set(ir.arg_regs)
5454
live = analyze_live_regs(ir.blocks, cfg)
5555
borrow = analyze_borrowed_arguments(ir.blocks, cfg, borrowed)
56-
defined = analyze_must_defined_regs(ir.blocks, cfg, args, values)
56+
defined = analyze_must_defined_regs(ir.blocks, cfg, args, values, strict_errors=True)
5757
ordering = make_value_ordering(ir)
5858
cache: BlockCache = {}
5959
for block in ir.blocks[:]:

0 commit comments

Comments
 (0)
0