docs: adapt Cython gotchas to this repo's actual idioms

bdraco · bdraco · commit e0e7541bfc34 · 2026-05-15T13:33:35.000-07:00
Trim and reshape the section added in the previous commit to
match python-zeroconf's reality rather than aioesphomeapi's:

- Drop the `noexcept` cdef bullet and the `except *` / `except? -N`
  bullet. The authored .pxd files in this repo don't use either —
  warning about traps the codebase doesn't reach is noise.
- Fix the `unsigned int` -&gt; `cdef int` bullet. DNS RR length and
  name compression pointer fields are 16-bit and 14-bit, so they
  don't cross the bit-31 signed boundary. Cite TTL instead — it
  is 32-bit (RFC 1035 §3.2.1) and the only field where the trap
  applies — and frame the rule as "stay with `unsigned int` end-
  to-end" since that is the convention `_protocol/incoming.pxd`,
  `_cache.pxd`, and `_handlers/*.pxd` already follow.
- Ground the `cdef`-typed module constants bullet in an actual
  example: `_ANSWER_STRATEGY_POINTER` in
  `_handlers/query_handler.{py,pxd}`.
- Ground the PyLong-conversion bullet in actual examples:
  `_TYPE_PTR`, `_DNS_PTR_MIN_TTL`, `_MIN_SCHEDULED_RECORD_EXPIRATION`
  already use the `cdef unsigned int` alias pattern.
- Drop "(things that have bitten us)" from the heading — most of
  the surviving bullets are preventative for this repo, not
  historical incidents.

Docs only.
diff --git a/CLAUDE.md b/CLAUDE.md
@@ -157,61 +157,51 @@ mutated from multiple threads without locks; no
   but the test matrix exercises 3.14t, so any new Cython module
   needs to keep working there.
 
-## Cython gotchas (things that have bitten us)
+## Cython gotchas
 
-These are non-obvious traps in the `.py` + `.pxd` setup that work
-fine in pure-Python mode but break or silently misbehave in the
-shipped Cython wheels. Tests pass locally with `SKIP_CYTHON=1`
-fallback paths, then CI on `use_cython` builds catches the issue —
-or worse, the issue ships and only manifests in production wheels.
+Non-obvious traps in the `.py` + `.pxd` setup that work fine in
+pure-Python mode but break or silently misbehave in the shipped
+Cython wheels. Distilled from the patterns that already exist in
+this repo's `.pxd` files and from incidents in sibling Cython-
+accelerated projects.
 
 - **`cdef`-typed module constants are not Python-importable.**
-  Declaring `cdef int _MAX_X` in `.pxd` makes Cython treat
-  `_MAX_X = 5` in the `.py` as a C int assignment; the Python
-  module dict never gets the binding. `from module import _MAX_X`
-  succeeds in pure-Python but raises `ImportError` under Cython.
-  **Pattern**: define both names — `MAX_X = 5` (Python-
-  importable) and `_MAX_X = MAX_X` (cdef-typed for hot-path
-  comparisons). Tests import the public name; production code
-  uses either.
-
-- **`noexcept` cdef paths must be pure C.** Calling a Python
-  method that can raise from inside a `cdef ... noexcept`
-  function is undefined / lossy — Cython prints the exception
-  via `WriteUnraisable` and silently continues. Keep `noexcept`
-  paths to sentinel returns and let the caller handle Python-
-  level work.
-
-- **`unsigned int` result returned through `cdef int` can flip
-  sign.** A length / offset decoded into an `unsigned int` and
-  returned via `cdef int` will come back negative for any value
-  with bit 31 set. If the caller does `if x < 0: return`, an
-  attacker-controlled large value silently hits the "incomplete"
-  / "stop processing" branch instead of being rejected. Either
-  cap the input range so decoded values stay in signed-int
-  range, or check explicit sentinel values (`x == _SENTINEL_A`)
-  instead of generic `< 0`. The on-the-wire parsers in
-  `_protocol/incoming` are exactly the kind of code where this
-  bites — name pointers and RR length fields cross the 16-bit /
-  32-bit signed boundary.
-
-- **`except *` / `except? -N` adds per-call exception checks.**
-  Switching a hot-path `cdef ... noexcept` to `except *` or
-  `except? -3` adds a `PyErr_Occurred()` check after every call.
-  Negligible for cold paths, measurable on hot paths — CodSpeed
-  has caught double-digit regressions from this on
-  packet-parsing benchmarks in sibling Cython projects. Prefer
-  `noexcept` for hot paths and route error handling through the
-  caller.
-
-- **Module-level Python int constants force `PyLong` conversion
-  in hot-path comparisons.** `if length > _MAX_FRAME_SIZE`
+  Declaring `cdef unsigned int _ANSWER_STRATEGY_POINTER` in
+  `query_handler.pxd` makes Cython treat
+  `_ANSWER_STRATEGY_POINTER = 1` in `query_handler.py` as a C int
+  assignment; the Python module dict never gets the binding.
+  `from zeroconf._handlers.query_handler import
+_ANSWER_STRATEGY_POINTER` succeeds in pure-Python but raises
+  `ImportError` under Cython. If you need the value visible from
+  Python (e.g. a test wants to assert on it), define both names —
+  a public `ANSWER_STRATEGY_POINTER = 1` Python binding plus a
+  `cdef`-typed `_ANSWER_STRATEGY_POINTER = ANSWER_STRATEGY_POINTER`
+  alias for hot-path comparisons.
+
+- **Match the existing `unsigned int` convention for length, TTL,
+  type/class, and offset fields.** `_protocol/incoming.pxd`,
+  `_cache.pxd`, and `_handlers/*.pxd` already declare these as
+  `unsigned int` end-to-end. Introducing a `cdef int` return that
+  carries a value originally decoded into `unsigned int` flips
+  sign for any value with bit 31 set — TTL is a 32-bit DNS field
+  (RFC 1035 §3.2.1, interpreted as unsigned), so a large TTL
+  passed back through a `cdef int` boundary becomes negative and
+  trips `< 0` sentinel branches. Stay with `unsigned int` across
+  the whole call chain; if you need a real sentinel, return an
+  explicit value (`UINT_MAX`, a dedicated constant) and check for
+  it by equality.
+
+- **Module-level Python int constants force `PyLong_AsLong` on
+  every hot-path comparison.** `if record.type == _TYPE_PTR`
   compiles to a Python attribute lookup + `PyLong_AsLong` per
-  call. Adding `cdef int _MAX_FRAME_SIZE` to the `.pxd` makes it
-  a native C comparison. Timing / size constants from `const.py`
-  used inside `cdef` hot paths in `_protocol/`, `_cache`,
-  `_handlers/`, or `_listener` should have a `cdef`-typed alias
-  declared in the appropriate `.pxd`.
+  call when `_TYPE_PTR` is just a `.py`-level binding. The repo
+  already follows the right pattern — `_cache.pxd` /
+  `record_manager.pxd` declare `cdef unsigned int _TYPE_PTR`,
+  `_DNS_PTR_MIN_TTL`, `_MIN_SCHEDULED_RECORD_EXPIRATION`, etc.
+  When adding a new size / TTL / type constant from `const.py`
+  to a `cdef` hot path in `_protocol/`, `_cache`, `_handlers/`,
+  or `_listener`, add the `cdef`-typed alias to the corresponding
+  `.pxd` at the same time.
 
 - **Sign-compare warnings in generated C are real.** `gcc`/
   `clang` warns when comparing `unsigned int` with `int` because
@@ -220,11 +210,11 @@ or worse, the issue ships and only manifests in production wheels.
   signedness of compared operands in the `.pxd` (e.g. if the
   local is `unsigned int`, declare the constant as
   `cdef unsigned int`; if the local is `int`, declare it
-  `cdef int`). The warning predicts a class of overflow bug like
-  the unsigned->signed trap above.
+  `cdef int`). The warning predicts the unsigned -> signed
+  overflow class of bug.
 
-- **CodSpeed regressions only show in the Cython build.** Pure-
-  Python (`SKIP_CYTHON=1`) tests can pass while production
+- **CodSpeed regressions only show up in the Cython build.**
+  Pure-Python (`SKIP_CYTHON=1`) tests can pass while production
   wire-format hot paths regress. Trust the CodSpeed check on PRs
   that touch any file in `TO_CYTHONIZE`; rebuild in place with
   `REQUIRE_CYTHON=1 poetry install --only=main,dev` before
