Commit 9eb71ad
committed
Drop body-exit NOP when except handler falls through open conditional
When a `with` body contains a try/except/else whose handler ends with
an open conditional (e.g. `except OSError: if not path.is_symlink(): raise`),
the handler does not unconditionally scope-exit and the synthetic
body-exit NOP introduced by `preserves_finally_entry_nop` is spurious
because control may still fall through to the with cleanup. Guard
against this by anding in `!statements_end_with_open_conditional_fallthrough`
when checking handler scope exits.
Also add the test
`test_with_try_except_else_open_conditional_handler_drops_body_exit_nop`
locking the pattern, and `fobj` to .cspell.dict/python-more.txt so the
prek cspell hook on CI clears for PR RustPython#7870 (used verbatim in the
already-committed test_nested_with_except_same_line_cleanup_threads_trampoline).1 parent 2e4055b commit 9eb71ad
2 files changed
Lines changed: 45 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -67,6 +67,7 @@ fillchar | |||
| 67 | 67 | fillvalue | |
| 68 | 68 | finallyhandler | |
| 69 | 69 | firstiter | |
| 70 | + fobj | ||
| 70 | 71 | firstlineno | |
| 71 | 72 | fnctl | |
| 72 | 73 | frombytes | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -765,7 +765,8 @@ impl Compiler { | |||
| 765 | 765 | && finalbody.is_empty() | |
| 766 | 766 | && handlers.iter().any(|handler| { | |
| 767 | 767 | let ast::ExceptHandler::ExceptHandler(handler) = handler; | |
| 768 | - Self::statements_end_with_finally_entry_scope_exit(&handler.body) | ||
| 768 | + !Self::statements_end_with_open_conditional_fallthrough(&handler.body) | ||
| 769 | + && Self::statements_end_with_finally_entry_scope_exit(&handler.body) | ||
| 769 | 770 | }) | |
| 770 | 771 | } | |
| 771 | 772 | _ => false, | |
@@ -16748,6 +16749,48 @@ def f(meta_path, cm): | |||
| 16748 | 16749 | ); | |
| 16749 | 16750 | } | |
| 16750 | 16751 | ||
| 16752 | + #[test] | ||
| 16753 | + fn test_with_try_except_else_open_conditional_handler_drops_body_exit_nop() { | ||
| 16754 | + let code = compile_exec( | ||
| 16755 | + "\ | ||
| 16756 | + def f(cm, self, path, OSError, now): | ||
| 16757 | + with cm: | ||
| 16758 | + try: | ||
| 16759 | + mtime = path.stat().st_mtime | ||
| 16760 | + except OSError: | ||
| 16761 | + if not path.is_symlink(): | ||
| 16762 | + raise | ||
| 16763 | + else: | ||
| 16764 | + self.assertGreaterEqual(path.stat().st_mtime, now) | ||
| 16765 | + ", | ||
| 16766 | + ); | ||
| 16767 | + let f = find_code(&code, "f").expect("missing f code"); | ||
| 16768 | + let ops: Vec<_> = f | ||
| 16769 | + .instructions | ||
| 16770 | + .iter() | ||
| 16771 | + .map(|unit| unit.op) | ||
| 16772 | + .filter(|op| !matches!(op, Instruction::Cache)) | ||
| 16773 | + .collect(); | ||
| 16774 | + | ||
| 16775 | + assert!( | ||
| 16776 | + !ops.windows(7).any(|window| { | ||
| 16777 | + matches!( | ||
| 16778 | + window, | ||
| 16779 | + [ | ||
| 16780 | + Instruction::Call { .. }, | ||
| 16781 | + Instruction::PopTop, | ||
| 16782 | + Instruction::Nop, | ||
| 16783 | + Instruction::LoadConst { .. }, | ||
| 16784 | + Instruction::LoadConst { .. }, | ||
| 16785 | + Instruction::LoadConst { .. }, | ||
| 16786 | + Instruction::Call { .. }, | ||
| 16787 | + ] | ||
| 16788 | + ) | ||
| 16789 | + }), | ||
| 16790 | + "try/except/else with open-conditional handler should fall directly into with cleanup, got ops={ops:?}", | ||
| 16791 | + ); | ||
| 16792 | + } | ||
| 16793 | + | ||
| 16751 | 16794 | #[test] | |
| 16752 | 16795 | fn test_elif_boolop_skips_following_elif_with_forward_jumpback_block() { | |
| 16753 | 16796 | let code = compile_exec( | |
0 commit comments