bpo-22807: Expose platform UUID generation safety information.#138
bpo-22807: Expose platform UUID generation safety information.#138warsaw merged 12 commits intopython:masterfrom
Conversation
| int &= ~(0xf000 << 64) | ||
| int |= version << 76 | ||
| self.__dict__['int'] = int | ||
| self.__dict__['is_safe'] = is_safe |
There was a problem hiding this comment.
Do you want to assert the type of is_safe ? There seem to extensive checking of the rest of the parameters.
Not that I imagine people creating UUID object in their own code, but is_safe feel also a lot like a Boolean.
There was a problem hiding this comment.
I'm not sure it's worth type checking is_safe. The other ones are checked to ensure sane, compliant UUIDs, but is_safe is just a flag with some extra information (which may or may not be helpful to an application). If someone wants to manually instantiate UUIDs with bogus is_safe flags, oh well.
|
|
||
| def __init__(self, hex=None, bytes=None, bytes_le=None, fields=None, | ||
| int=None, version=None): | ||
| int=None, version=None, is_safe=SafeUUID.unknown): |
There was a problem hiding this comment.
I changed my mind on whether this should affect older versions of Python.
If so, you can also consider making it kwarg-only. As you are already using is as a kwarg. Not sure if that's useful, or worth it. It's may also be a question of taste, and leaving it as is seem perfectly fine.
There was a problem hiding this comment.
Thanks for the review! I did think about that, and it might indeed make sense, but it's a matter of style in this case, and I wasn't sure I wanted to introduce that to the API. But on thinking about it again now that you bring it up, I does make sense.
Carreau
left a comment
There was a problem hiding this comment.
Thanks for the review!
My pleasure !
I did think about that, and it might indeed make sense, but it's a matter of style in this case, and I wasn't sure I wanted to introduce that to the API. But on thinking about it again now that you bring it up, I does make sense.
Happy to be of some use :-)
berkerpeksag
left a comment
There was a problem hiding this comment.
I just left some comments, but the PR looks good to me.
| .. class:: UUID(hex=None, bytes=None, bytes_le=None, fields=None, int=None, version=None) | ||
| .. class:: SafeUUID | ||
|
|
||
| .. attribute:: SafeUUID.safe |
There was a problem hiding this comment.
I know this is the current style in uuid.rst, but it might be better use the following style in new documentation:
.. class:: SafeUUID
.. versionadded:: 3.7
.. attribute:: safe
The UUID was generated by the platform in a multiprocessing-safe way.
...There was a problem hiding this comment.
Yeah, that looks nice, thanks. The one problem is that SafeUUID still gets labeled as a class in the HTML. Okay, technically it is a class, but it would be nice to have an enum:: markup. I grep'd around in the source and couldn't find any such examples, so I don't know if we support that yet.
There was a problem hiding this comment.
You're right. We don't have an enum directive now, but it should be easy to add one. You can add me to nosy list if you create an issue on bugs.p.o :)
| equal(((u.clock_seq_hi_variant & 0x3f) << 8) | | ||
| u.clock_seq_low, 0x3fff) | ||
|
|
||
| @unittest.skipUnless(importable('ctypes'), 'requires ctypes') |
There was a problem hiding this comment.
Nitpick: I'd flip the order of the decorators.
| # unknown (unless I suppose the platform is buggy). | ||
| self.assertNotEqual(u.is_safe, uuid.SafeUUID.unknown) | ||
|
|
||
| @unittest.skipUnless(importable('ctypes'), 'requires ctypes') |
There was a problem hiding this comment.
Just a suggestion: Using self.subTest() would make the following four tests much shorter and IMO easier to understand.
There was a problem hiding this comment.
I've played a bit with subTest() and I'm not entirely thrilled with it. AFAICT, subtests only print output when they fail, not when they succeed. In verbose mode, I'd want to see all 4 tests printed individually. (That's one of the things I'd like to eventually fix.) For now, I think I'll leave them as separate tests.
|
|
||
| .. versionadded:: 3.7 | ||
|
|
||
| .. class:: UUID(hex=None, bytes=None, bytes_le=None, fields=None, int=None, version=None, *, is_safe=SafeUUID.unknown) |
There was a problem hiding this comment.
Currently, the body of UUID() documentation briefly describes all its arguments so I think we can just copy is_safe documentation in Lib/uuid.py
``local.getlocale(locale.LC_CTYPE)`` and ``locale.getpreferredencoding(False)`` may give different answers in some cases (such as the ``en_IN`` locale). ``re.LOCALE`` uses the latter, so update the test case to match.
Replace STACKLESS_PROPOSE(func) in do_call() and ext_do_call() by STACKLESS_PROPOSE_ALL().
- Initialize variable 'len' in function unwrap_frame_arg(). - Assign to '*valid' in function slp_find_execname(). - Disable GCC warning -Waddress in macro TASKLET_SETVAL(task, val). - Update Stackless/changelog.txt for issues python#138, python#140
Replace STACKLESS_PROPOSE(func) in do_call() and ext_do_call() by STACKLESS_PROPOSE_ALL(). (cherry picked from commit ae3d7a2)
- Assign to '*valid' in function slp_find_execname(). - Disable GCC warning -Waddress in macro TASKLET_SETVAL(task, val). - Update Stackless/changelog.txt for issues python#138, python#140 (cherry picked from commit 3461ccf)
…ary domains W-PROTOCOL-CODIFY Phase A per supervisor 20:03:34Z directive (post pythia python#138 python#3 + supervisor 19:37:39Z STRONG CONCUR + theologian 19:37:17Z spec). Replaces named-only conversions (post W-2B-RECONVERT 3-class fix at 8e0c73e) with single-field struct typedefs that the C compiler tracks distinctly. Inline arithmetic at the seam was the bug class, named conversions were the WEAKER form (HirType negative control: 14 reinterpret_casts shipped broken until 2026-04-15 because naming degrades to advisory). PhxMem has_base structural flag is the POSITIVE control: JIT_CHECK_C didn't fire on dual-arch gate; structural prevention worked. CHANGES (Python/jit/bytecode_c.h): - typedef struct { int v; } BcByteOffset; - typedef struct { int v; } BcInstrIndex; - phx_bc_offset_to_instr_index now takes BcByteOffset, returns BcInstrIndex - phx_bc_instr_index_to_offset now takes BcInstrIndex, returns BcByteOffset - Header comment cites P5 origin + paired empirical controls CHANGES (Python/jit/hir/builder_emit_c.c, build_inline_except_opcode_array_c): - Class A site: wrap except_body_offset in BcByteOffset, convert to BcInstrIndex, unwrap .v for jit_bc_instr_init (existing raw-int API) - Class C site: wrap jit_bc_instr_base_offset return in BcInstrIndex, convert to BcByteOffset, unwrap .v for entry->base_offset - Class B site: wrap jit_bc_instr_get_jump_target return in BcInstrIndex, convert to BcByteOffset, unwrap .v for phx_block_map_lookup_or_panic The wrap/unwrap dance localizes the .v access to exactly the boundary between raw-int existing APIs and typed conversion functions. Compiler will catch any future cross-domain assignment (e.g., assigning BcByteOffset to BcInstrIndex without conversion) as type error — the structural property the spec requires. Spec: docs/w-protocol-codify-spec.md P5 (lines 109-136). Behavior: ZERO change. The numeric values are identical to the named- conversion fix at 8e0c73e (same arithmetic, same call ordering). The 30/30 sentinels (test_exc_raise_catch + 3 W-2B-RECONVERT sentinels) + test_multiple_exceptions_in_loop must remain 30/30 PASS. OUT-OF-SCOPE for Phase A (per spec): - Other domains (cache-id, ref-kind, type-kind) — apply pattern as encountered, not pre-authored. - Existing C++ BCOffset/BCIndex code — already type-enforced in C++. - jit_bc_instr_* APIs — keep raw int signatures; conversion happens at call sites in helpers that cross domains. Future Phase: if a new helper repeatedly wraps/unwraps the same value, consider promoting the return type to BcInstrIndex / BcByteOffset. Phase B (gate integration) + Phase C (per-conversion P6/P7 application to emitAnyCall + emitLoadMethodStatic) queued post-Phase-A. Verification pending: testkeeper rebuild + 30x W-2B-RECONVERT sentinel suite + 30x test_multiple_exceptions_in_loop + full Phoenix gate + ABBA + dual-arch.
Per supervisor 20:39:03Z directive (post pythia python#138 python#2: push 55 was the first commit in 'ACTIVE-dispatch territory' — Class A unit bug previously masked dispatch-loop coverage on multi-except paths via interpreter fallback for the entire push-54 lifetime). Adds 4 sentinels covering exception-handling shapes not exercised by the W-2B-RECONVERT sentinels (which targeted Class A/B/C latent fixes): 1. test_exc_try_else — no-raise path + else branch. Exercises the no-fire route through emitInlineExceptionMatch fast-path + JIT_EVAL falls through to else block. 2. test_exc_try_finally_with_raise — nested try/finally inside try/except with raise(). The raise-CALL exercises emitCallExceptionHandler's D1 stack-cleanup pre-amble + D3 result re-push invariant when an inner finally runs while the exception propagates to the outer try. 3. test_exc_nested_try_inner_catches — nested try where inner except catches and outer block continues normally. Exercises FrameState depth-trim correctness across nested handler scope transitions. 4. test_exc_raise_in_handler — re-raise a DIFFERENT exception type from inside an except handler. The new raise() inside except is itself a CALL (constructor of the new exception class), so emitCallExceptionHandler fires while we're already in handler dispatch — tests nested handler activation. Tests 2-4 actively exercise emitCallExceptionHandler dispatch. Test 1 covers emitInlineExceptionMatch no-fire path. Together they cover the four major dispatch shapes not exercised by W-2B-RECONVERT sentinels. Per theologian 20:40:59Z patch-shape APPROVE. Verification pending: testkeeper 30x each + regression check on existing 5 W-2B-RECONVERT sentinels (helper unchanged at push 56). If all 4 PASS → C dispatch path is sound for these shapes; W-2A-DISPATCH- COVERAGE complete; pivot to W-PROTOCOL-CODIFY Phase B per roadmap. If any FAIL → new bug discovery; investigate per HIR-diff Phase 0 methodology (compare to push-53 ebec018 reference for the failing shape per theologian 20:40:59Z).
Add an enum
SafeUUIDwhich describes whether the platform has generated a UUID in a safe manner (i.e. ensuring that no two processes can generate the same UUID). Also addUUID.is_safeattribute which relays the information from the underlying platform.