Skip to content

bpo-29919: Remove unused imports found by pyflakes#137

Merged
vstinner merged 1 commit intopython:masterfrom
vstinner:unused_imports
Mar 27, 2017
Merged

bpo-29919: Remove unused imports found by pyflakes#137
vstinner merged 1 commit intopython:masterfrom
vstinner:unused_imports

Conversation

@vstinner
Copy link
Copy Markdown
Member

@vstinner vstinner commented Feb 16, 2017

Make also minor PEP8 coding style fixes on modified imports.

http://bugs.python.org/issue29919

Copy link
Copy Markdown
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

Please open an issue on the tracker. This is enough large change.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

setUpModule should be imported in the module namespace!

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

fixed

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Same as above.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

fixed

Comment thread Lib/unittest/__main__.py Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This looks suspicious. Are you sure that TestProgram is not needed?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Lib/unittest/main.py ends with "main = TestProgram". And TestProgram is not used in Lib/unittest/main.py.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why BooleanVar and StringVar are commented out?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

They are used in commented code a few lines below. I don't want to remove this commented code, since I don't know this file.

@orsenthil
Copy link
Copy Markdown
Member

Here was my basis for approval. The change is an automated one using pyflakes and our unit-tests are successful. I could not spot anything critical after looking at the diff. ( That is, if things would have broken by removal of top-level imports, then we should have seen when the module was invoked).

@warsaw
Copy link
Copy Markdown
Member

warsaw commented Feb 17, 2017

Gosh I'd love to enable flake8 on the stdlib, since it often causes my eyes to bleed red when I open a py file from the stdlib. But I suppose that will just cause unnecessary code churn. It's still nice to clean things up when you can, like unused imports.

@vstinner
Copy link
Copy Markdown
Member Author

vstinner commented Feb 18, 2017 via email

@serhiy-storchaka serhiy-storchaka added the type-feature A feature request or enhancement label Feb 18, 2017
@vstinner vstinner changed the title Remove unused imports found by pyflakes bpo-29919: Remove unused imports found by pyflakes Mar 27, 2017
Make also minor PEP8 coding style fixes on modified imports.
@vstinner
Copy link
Copy Markdown
Member Author

I rebased my patch and removed the two changes removing setUpModule() :-)

@vstinner
Copy link
Copy Markdown
Member Author

@serhiy-storchaka: "Please open an issue on the tracker. This is enough large change."

I don't see the value added by creating an issue, but ok, I created http://bugs.python.org/issue29919 :-)

@vstinner vstinner merged commit d6debb2 into python:master Mar 27, 2017
@vstinner
Copy link
Copy Markdown
Member Author

Thanks for the reviews!

@vstinner vstinner deleted the unused_imports branch March 27, 2017 14:05
terryjreedy added a commit that referenced this pull request Jun 11, 2017
Part of patch by Victor Stinner.
(cherry-pick from d6debb2)
akruis pushed a commit to akruis/cpython that referenced this pull request Oct 22, 2017
Add Stackless support to all call-API-functions in abstract.c
akruis pushed a commit to akruis/cpython that referenced this pull request Oct 22, 2017
Add Stackless support to _PyObject_FastCallKeywords() and
_PyFunction_FastCallKeywords.
akruis pushed a commit to akruis/cpython that referenced this pull request Oct 29, 2017
Add Stackless support to _PyObject_Call_Prepend().
akruis pushed a commit to akruis/cpython that referenced this pull request Oct 29, 2017
- Add Stackless support to _PyObject_FastCallKeywords().
- Use STACKLESS_PROPOSE_... in a consistent way.
akruis pushed a commit to akruis/cpython that referenced this pull request Oct 29, 2017
Add Stackless support to _PyCFunction_FastCallKeywords().
akruis pushed a commit to akruis/cpython that referenced this pull request Oct 29, 2017
Add Stackless support for METH_FASTCALL C-functions.
Add a warning about the ABI change to the Stackless changelog.
akruis pushed a commit to akruis/cpython that referenced this pull request Oct 29, 2017
akruis pushed a commit to akruis/cpython that referenced this pull request Mar 25, 2018
Add Stackless support to _PyObject_CallFunctionVa() and related
functions.
akruis pushed a commit to akruis/cpython that referenced this pull request Mar 25, 2018
Fix Stackless support of _PyCFunction_FastCallKeywords().
akruis pushed a commit to akruis/cpython that referenced this pull request Mar 25, 2018
Add Stackless support to _PyMethodDescr_FastCallKeywords().
akruis pushed a commit to akruis/cpython that referenced this pull request Mar 25, 2018
Add Stackless support to cfunction_call_varargs().
akruis pushed a commit to akruis/cpython that referenced this pull request Mar 25, 2018
Add Stackless support to function _PyObject_FastCall_Prepend().
jaraco pushed a commit that referenced this pull request Dec 2, 2022
SonicField added a commit to SonicField/cpython that referenced this pull request Apr 25, 2026
…_inline_except_opcode_array_c

Fixes THREE boundary-domain bugs in build_inline_except_opcode_array_c
(introduced W27c #2a 7135d94), found across two HIR-diff Phase 0 cycles
of the W-2B-RECONVERT investigation.

CONVENTION (Python/jit/bytecode.cpp:8-14, builder.cpp:1235,
phx_frame_state.h cur_instr_offs semantics):
- jit_bc_instr_init expects INSTRUCTION INDEX (codeUnit[])
- jit_bc_instr_get_jump_target / next_offset / base_offset return whatever
  was stored at init (now INDEX after Class A fix)
- phx_block_map keys are BYTE OFFSETS
- OpcodeArrayEntry.base_offset is consumed downstream as BYTE OFFSET
  (cur_instr_offs assignment per builder_emit_c.c:3320)
- BCOffset.value() (caller-passed except_body_offset) is BYTE OFFSET
- BYTES = INDEX * sizeof(_Py_CODEUNIT) (= 2 in 3.12)

NAMED CONVERSIONS (Python/jit/bytecode_c.h, gated by _Py_OPCODE):
  static inline int phx_bc_offset_to_instr_index(int byte_off);
  static inline int phx_bc_instr_index_to_offset(int instr_idx);

Codifies the boundary-domain rule by example (per pythia python#137 python#2 +
supervisor 19:01:19Z + theologian 19:01:17Z).

THREE FIXES:

CLASS A (line 3241): jit_bc_instr_init was passed except_body_offset
(BCOffset.value() byte offset) where INSTRUCTION INDEX was expected.
codeUnit(code)[byte_offset] read PAST end of co_code → garbage opcode →
switch-default → Deopt with corrupt frame state. Found by Phase 0
HIR-diff (test_exc_raise_catch bb 12: correct Return -1 vs corrupt
LoadConst NoneType + Deopt at offset 58).

CLASS B (line 3273-3275, theologian class-of-bug audit 18:42:53Z):
target = jit_bc_instr_get_jump_target returns INDEX, but
phx_block_map_lookup_or_panic expects BYTE OFFSET. Without conversion,
JUMP_BACKWARD-in-except-body lookup fails: JIT_CHECK_C panic OR silent
wrong-block. Dormant pre-fix because no test had backward-jump-in-except-body.

CLASS C (line 3260, exposed by Phase 0' HIR-diff after Class A+B fix):
After Class A fix corrected the init to INDEX, jit_bc_instr_base_offset
returns INDEX. But entry->base_offset is consumed downstream (line 3320)
as BYTE OFFSET via match_tc.frame.cur_instr_offs assignment. Pre-fix
'correct by accident' — Class A's BYTES-as-INDEX init wrote BYTES into
bci->base_offset, so jit_bc_instr_base_offset returned BYTES, matching
downstream. Correct Class A exposed Class C: cur_instr_offs got INDEX
(half the correct BYTE value) → interpreter Deopt resumed at wrong
bytecode position → SIGSEGV in test_multiple_exceptions_in_loop
(deterministic 0/20 post Class A+B fix, vs 20/20 PASS pre-W27c).

DIAGNOSIS:
HIR-diff for test_multiple_exceptions_in_loop revealed Deopt CurInstrOffset
124 (correct, BYTES) → 62 (wrong, INDEX = 124/2). Direct evidence of the
domain mismatch.

LATENT in pushed W27c #2a (e4e7507 on SonicField/cpython): all three
classes present. Class A, B dormant (no test exercises emitInlineExceptionMatch
or JUMP_BACKWARD-in-except-body). Class C compensated by Class A — both
broken in opposite directions canceling out for downstream consumers of
entry->base_offset. ALL three must fix together.

INVESTIGATION CHAIN:
- testkeeper bisect 17:52:30Z localized #2b regression → W27c #2b sole
- pythia python#136 python#1 18:23:34Z flagged HEAP/RACE rode on absence-of-evidence
- generalist 18:24Z proposed HIR-diff Phase 0 falsifier
- generalist 18:32+18:34Z captured HIR_2a + HIR_2b dumps; found Class A
- theologian 18:42:53Z class-of-bug audit found Class B
- supervisor 18:43:17Z directed dual-fix
- pythia python#137 python#2 19:00:29Z flagged inline-arithmetic violates boundary-domain rule
- supervisor 19:01:19Z + theologian 19:01:17Z directed amend to named conversions
- testkeeper 19:06:55Z full Phoenix gate caught NEW regression
  (test_multiple_exceptions_in_loop deterministic 0/20)
- generalist 19:14Z HIR-diff Phase 0' on test_multiple_exceptions_in_loop
  revealed Class C (cur_instr_offs 124→62)
- supervisor 19:16:42Z authorized Class C fix + extended audit

OTHER OpcodeArrayEntry FIELDS AUDITED (per supervisor 19:16:42Z extended
class-of-bug discipline):
- entry->opcode: written from jit_bc_instr_opcode (no domain — opcode value);
  consumed in dispatch loop switch. CLEAN.
- entry->oparg: written from jit_bc_instr_oparg (no domain — oparg value);
  consumed in dispatch loop emit calls. CLEAN.
- entry->base_offset: Class C above; FIXED.
- entry->const_obj: written from PyTuple_GET_ITEM (PyObject*); consumed in
  dispatch loop hir_type_from_object. CLEAN (no domain conversion).
- entry->jump_target_block: Class B above; FIXED.

VERIFICATION pending (testkeeper 4-suite extended verify):
1. 30x test_exc_raise_catch (Class A regression)
2. 30x test_exc_binary_subscr_dict_in_try (Class A latent activation)
3. 30x test_exc_continue_in_loop (Class B latent activation)
4. 30x multi-except-in-loop sentinel (Class C latent activation)
5. Full Phoenix suite (which originally caught Class C)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type-feature A feature request or enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants