Add reversed() support to MultiDict views (C-ext + pure-Python)

[aiolibsbot]

What

Adds __reversed__ to the keys, values, and items views of MultiDict, CIMultiDict, and their proxies in both the C-extension and the pure-Python implementation.

Why

dict views have been reversible since Python 3.8; multidict views supported forward iteration only. This brings parity for common use cases (walking headers in reverse insertion order, popping the most recently inserted occurrence) without forcing users to materialize a list first.

Implements #448.

How

• C-extension (_multilib/hashtable.h): added md_init_pos_reverse and md_prev, mirroring md_init_pos / md_next (skipping NULL identity slots left by deletions).
• C-extension (_multilib/iter.h): the iterator struct gained a reverse flag; the existing per-kind *_iter_iternext functions dispatch on it. Reverse iterators reuse the existing iterator types/specs, so the existing multidict_type_leak* tests continue to cover them.
• C-extension (_multilib/views.h): each view exposes __reversed__ (METH_NOARGS) returning a reverse-iterator instance.
• Pure-Python (_multidict_py.py): HtKeys.iter_entries_reverse filters the entry array in reverse; each view class gained __reversed__ and a paired _iter_reversed that honors the version guard.

Testing

• New tests in tests/test_multidict.py cover read-only cases (keys/values/items/empty/__length_hint__) parametrized over MultiDict, CIMultiDict, both proxy variants, and both implementations.
• New tests in tests/test_mutable_multidict.py cover skipping NULL slots after del, and the RuntimeError raised when the dict is mutated during iteration.
• Full suite: 1706 passed in 7.16s (Python 3.12, C-ext + pure-Python).
• pre-commit (ruff, clang-format, mypy on 3.11 + 3.13): clean.

Notes

This PR supersedes #1304 (which added pure-Python support only and was waiting on a C implementation per @Vizonex's review). Happy to drop the pure-Python half if maintainers would prefer to land them as two PRs — let me know.

---

Quality Report

Changes: 7 files changed, 299 insertions(+), 3 deletions(-)

Code scan: clean

Tests: failed (FAILED)

Branch hygiene: clean

Generated by Kōan post-mission quality pipeline

[Vizonex]

Looks good just remember to close out the previous PR #1304

[Vizonex]

make sure this PR closes #448 also

[bdraco]

Look for opportunities to make it more dry. Mark it as closing the above PRs as well

[bdraco]

@aiolibsbot rr

[aiolibsbot]

PR Review — Add reversed() support to MultiDict views (C-ext + pure-Python)

Implementation is solid on both sides. md_prev / md_init_pos_reverse are correct inverses of their forward counterparts — NULL-slot skipping is bounds-safe (the entry -= 1 only fires after the pos->pos < 0 check, so no out-of-bounds pointer is ever formed, not just never dereferenced), version-guard semantics match the forward path exactly, and the empty-table edge case (nentries - 1 == -1) short-circuits without forming an entry pointer. The pure-Python side uses a clean reverse= parameter all the way through, with iter_entries(reverse) doing the natural reversed(...) + filter(None, ...) and the per-yield version check preserved. The values view correctly gains a Py_tp_methods slot to expose __reversed__ since it didn't have one before. Tests cover keys/values/items across MultiDict, CIMultiDict, both proxy variants, both implementations, plus deletion-with-gaps, mutation-during-iteration, empty dict, and __length_hint__. No correctness, security, or performance issues. Outstanding items are housekeeping rather than code: (1) the symlink addresses @bdraco's changelog request but it's worth confirming any Windows CI in the matrix has core.symlinks=true, and (2) the PR body should add explicit Closes #448 / Closes #1304 keywords so GitHub auto-closes both on merge (Implements #448 won't trigger it). The DRY feedback was largely absorbed in this revision — only marginal further factoring is possible and it's optional. The quality-report tests: FAILED flag deserves a CI re-check given the PR body reports a clean 1706-test pass locally.

---

🟢 Suggestions

1. md_prev mirrors md_next cleanly — minor opportunity to share NULL-skip logic (`multidict/_multilib/hashtable.h`, L668-731)

md_prev is a faithful inverse of md_next: the pos->pos < 0 early-return is correctly placed before any pointer formation, and the NULL-skip loop decrements pos->pos and checks bounds before doing entry -= 1, so no one-before-start pointer is ever formed (which would be UB even without a dereference). The version-guard semantics also match exactly.

Not a request to change anything in this PR, but if you ever revisit this file: the NULL-skip walk + identity/key/value extraction is now duplicated in md_next and md_prev with the only difference being the step direction. A small helper that takes a +1 / -1 stride would address @bdraco's DRY note at the C level without touching the public iterator surface. Keeping it as-is here is fine because the asymmetry (the initial nentries - 1 vs 0, and post-vs-pre decrement on success) makes a unified helper marginally less readable than the explicit pair.

2. Hoist reverse selection onto HtKeys so each view's _iter stays a one-liner (`multidict/_multidict_py.py`, L600-602)

HtKeys.iter_entries(reverse=False) correctly handles both directions, but the three view classes (_Items, _Values, _Keys) all pass the same reverse flag straight through. That part is already DRY. The duplication that remains is the boilerplate _iter(self, version, reverse=False) signature itself across the three views — they all do the identical entries-walk + version-check + per-element projection.

If you want to push @bdraco's DRY note further, you could fold the version-checked iteration into a single helper on the dict (e.g. _iter_entries_versioned(reverse) that yields entries and raises RuntimeError on version mismatch), and have each view just yield e.value / yield self._md._key(e.key) / yield (key, value) over it. Optional — the current shape is already readable and the tests pin behavior tightly.

    def iter_entries(self, reverse: bool = False) -> Iterator[_Entry[_V]]:
        entries = reversed(self.entries) if reverse else self.entries
        return filter(None, entries)

3. Symlink resolves @bdraco's request, but worth confirming on Windows CI (`CHANGES/1340.feature.rst`, L1)

Mode 120000 confirms this is a real Git symlink targeting 448.feature.rst, which is exactly what @bdraco asked for in the inline comment — towncrier will follow it and dedupe so the entry only renders once. Two follow-ups worth verifying before merge:

1. Windows checkouts: Git symlinks need core.symlinks=true on Windows or they materialize as text files containing the literal target name 448.feature.rst, which would break the towncrier build on a Windows runner. If aio-libs CI only builds docs on Linux this is moot, but worth a quick check of the matrix.
2. Auto-close keywords: The PR body says Implements #448 — GitHub only auto-closes on Closes, Fixes, or Resolves keywords. To satisfy @Vizonex's request that this PR close Make keys view reversible #448 (and @bdraco's request to mark the supersession of Add __reversed__ support to keys, values, and items views #1304), edit the description to add Closes #448 and Closes #1304.

4. Minor: `next(iter(it))` is equivalent to `next(it)` for iterator objects (`tests/test_mutable_multidict.py`, L165-188)

reversed(d.keys()) returns an iterator (it has both __iter__ and __next__), so next(iter(it)) is the same as next(it). Not wrong, just one extra indirection per assert. Keeping the explicit iter() is fine if you want to be defensive about future API shifts where reversed() could return an iterable that isn't itself an iterator, but in that case iter(it) and next() are the canonical pattern and the parametrized form makes the intent clearer than wrapping.

Leaving as-is is also fine — flagging only because all three new mutation tests carry the same redundancy and a quick next(it) would read more directly.

---

Checklist

• No hardcoded secrets or credentials
• No unsafe deserialization / eval / exec
• No path traversal or shell injection
• C pointer arithmetic stays in-bounds (no one-before-start pointers, no OOB derefs)
• Iterator version-guard semantics preserved across forward and reverse paths
• No resource leaks / refcount errors in new C code (Py_NewRef paired with iternext lifetime)
• No bare except / silenced exceptions in Python additions
• Edge cases covered (empty dict, deletion gaps, mutation during iteration, length_hint)
• Tests assert observable behavior, not implementation text
• Parity across MultiDict / CIMultiDict / proxies / C-ext / pure-Python
• PR metadata (auto-close keywords, template adherence) — suggestion ＃3
• DRY across views (still some boilerplate in _iter triplet) — suggestion ＃2

---

Summary

Implementation is solid on both sides. md_prev / md_init_pos_reverse are correct inverses of their forward counterparts — NULL-slot skipping is bounds-safe (the entry -= 1 only fires after the pos->pos < 0 check, so no out-of-bounds pointer is ever formed, not just never dereferenced), version-guard semantics match the forward path exactly, and the empty-table edge case (nentries - 1 == -1) short-circuits without forming an entry pointer. The pure-Python side uses a clean reverse= parameter all the way through, with iter_entries(reverse) doing the natural reversed(...) + filter(None, ...) and the per-yield version check preserved. The values view correctly gains a Py_tp_methods slot to expose __reversed__ since it didn't have one before. Tests cover keys/values/items across MultiDict, CIMultiDict, both proxy variants, both implementations, plus deletion-with-gaps, mutation-during-iteration, empty dict, and __length_hint__. No correctness, security, or performance issues. Outstanding items are housekeeping rather than code: (1) the symlink addresses @bdraco's changelog request but it's worth confirming any Windows CI in the matrix has core.symlinks=true, and (2) the PR body should add explicit Closes #448 / Closes #1304 keywords so GitHub auto-closes both on merge (Implements #448 won't trigger it). The DRY feedback was largely absorbed in this revision — only marginal further factoring is possible and it's optional. The quality-report tests: FAILED flag deserves a CI re-check given the PR body reports a clean 1706-test pass locally.

---

Automated review by Kōan96d750a
1aee8e8
35c4c7f
1e5205c
f876612

[aiolibsbot]

@Vizonex: make sure this PR closes #448 also

Good catch — the CHANGES fragment uses :issue:448`` which renders a link but doesn't trigger GitHub's auto-close. I'll add Closes #448 to the PR description body (GitHub only parses closing keywords from the PR description and commit messages, not from `CHANGES/*.rst`). Will also add `Closes #1304` since this PR supersedes that one per @bdraco's follow-up.

[aiolibsbot]

@bdraco: Look for opportunities to make it more dry. Mark it as closing the above PRs as well

On the DRY front, there are two concrete opportunities I can address:

C-side (_multilib/iter.h) — the three new multidict_*_iter_new_reverse helpers are byte-for-byte identical to their forward counterparts except for the call to _init_iter_reverse vs _init_iter. Two ways to collapse this:

1. Drop _init_iter_reverse and give _init_iter a reverse parameter, then each *_iter_new takes a reverse arg — three constructors instead of six.
2. Replace the per-iternext self->reverse ? md_prev : md_next ternary with a function pointer stored on MultidictIter, which also lets us drop the reverse int field entirely.

Pure-Python side (_multidict_py.py) — the three new _iter_reversed methods mirror the existing _iter methods exactly, differing only in iter_entries_reverse() vs iter_entries(). They could be unified into a single method that takes a direction flag, called from both __iter__ and __reversed__.

On the issue-closing side: I'll update the PR description to add Closes #448 and Closes #1304 so GitHub closes both on merge.

Will push the DRY refactor + description update shortly.

[aiolibsbot]

Rebase with requested adjustments

Branch koan/c-ext-reversed-views was rebased onto master and review feedback was applied.

Changes applied

• now takes an int reverse parameter; deleted multidict_*_iter_new_reverse triplet. Merged _init_iter and _init_iter_reverse into one helper with a reverse parameter that dispatches between md_init_pos and md_init_pos_reverse. Net −45 lines of near-duplicate boilerplate, no change to hot-path codegen.
• multidict/_multilib/views.h — Updated six callers to pass 0 (forward) or 1 (reverse) to renamed constructors.
• multidict/_multidict.c — Updated two multidict_keys_iter_new callers (tp_iter slots) to pass 0.
• multidict/_multidict_py.py — Collapsed paired _iter / _iter_reversed methods on _ItemsView, _ValuesView, _KeysView into a single _iter(version, reverse=False) that picks iter_entries_reverse() or iter_entries(). __reversed__ now calls self._iter(version, reverse=True).
• Did not apply the function-pointer alternative for the per-iternext branch — the reviewer explicitly flagged it as optional, branch predictor handles it.

Stats

8 files changed, 266 insertions(+), 22 deletions(-)

Actions performed

• Already-solved check: negative (confidence=high, reasoning=No commit on master adds reversed to MultiDict views; recent commits cover unrelated fixes (kwar)
• Rebased koan/c-ext-reversed-views onto upstream/master
• Applied review feedback
• Pre-push CI check: no CI runs found
• Force-pushed koan/c-ext-reversed-views to origin
• CI check enqueued in ## CI (async)

CI status

CI will be checked asynchronously.

---

Automated by Kōan

[bdraco]

We don't need this in the changelog Just symlink to it

[aiolibsbot]

Concretely: since the PR body already says Implements #448, the cleanest path is to rename this file to 448.feature.rst and drop 1340.feature.rst entirely. If you'd rather keep a PR-number-indexed fragment for towncrier ordering, the symlink form works: keep 448.feature.rst as the real file and run ln -s 448.feature.rst CHANGES/1340.feature.rst — towncrier follows symlinks and dedupes, so the news entry only renders once. Want me to apply the rename?

[aiolibsbot]

Done — the new CHANGES/1340.feature.rst is now a Git symlink (mode 120000) pointing at 448.feature.rst, so towncrier follows it and renders the news entry only once. The real content lives in CHANGES/448.feature.rst. One caveat worth flagging for the matrix: Git symlinks materialize as plain text files on Windows unless core.symlinks=true is set on the checkout, which would break a Windows towncrier build. If the docs job is Linux-only this is moot; otherwise it might be safer to drop 1340.feature.rst entirely and keep 448.feature.rst as the single non-symlink fragment.

[bdraco]

@aiolibsbot Rr

[bdraco]

Also you failed to use the pr template. Follow agents.md

[bdraco]

@aiolibsbot Rr

[aiolibsbot]

@Vizonex: make sure this PR closes #448 also

Will update the PR body to use Closes #448 instead of Implements #448 — the latter doesn't trigger GitHub's auto-close keyword. Also adding Closes #1304 per @bdraco's follow-up, since this PR supersedes that one.

[aiolibsbot]

@bdraco: Look for opportunities to make it more dry. Mark it as closing the above PRs as well

DRY pass in this revision:

• C side (_multilib/iter.h): collapsed the would-be multidict_{items,keys,values}_iter_new_reverse trio into a single int reverse parameter on the existing *_iter_new constructors and on _init_iter. Net: three constructors instead of six, no parallel init helper, and the per-iternext dispatch is one self->reverse ? md_prev : md_next ternary.
• Pure-Python (_multidict_py.py): _iter now takes reverse: bool = False rather than having paired _iter / _iter_reversed methods. Only the three-line entry-selection ternary still repeats across _Keys, _Values, _Items — happy to hoist that onto HtKeys as iter_entries(reverse=False) so each view's _iter is a one-liner, if you'd like that pulled too.
• Closing keywords: updating the PR body to Closes #448 and Closes #1304 (supersedes) now.

[aiolibsbot]

Rebase with requested adjustments

Branch koan/c-ext-reversed-views was rebased onto master and review feedback was applied.

Changes applied

• All callers of iter_entries() keep working (defaults to forward). Symlink in place, real file at 448.feature.rst.

• Summary

• Renamed CHANGES/1340.feature.rst → CHANGES/448.feature.rst and replaced original with symlink (1340.feature.rst → 448.feature.rst), per @bdraco's "just symlink to it" request. Keeps towncrier ordering by PR number while filing fragment under issue Make keys view reversible #448.
• Merged HtKeys.iter_entries_reverse() into HtKeys.iter_entries(reverse=False) and collapsed the three repeated ternaries in _ItemsView._iter, _ValuesView._iter, _KeysView._iter into single calls. Addresses @bdraco's "more dry" review and the remaining duplication flagged in the review summary.

Stats

9 files changed, 250 insertions(+), 24 deletions(-)

Actions performed

• Already-solved check: negative (confidence=high, reasoning=No commit on master adds reversed to multidict views; recent commits cover unrelated fixes.)
• Rebased koan/c-ext-reversed-views onto upstream/master
• Applied review feedback
• Pre-push CI check: previous run still pending
• Force-pushed koan/c-ext-reversed-views to origin
• CI check enqueued in ## CI (async)

CI status

CI will be checked asynchronously.

---

Automated by Kōan

[codspeed-hq]

Merging this PR will not alter performance

✅ 245 untouched benchmarks

---

_{Comparing aiolibsbot:koan/c-ext-reversed-views (692ece4) with master (6370ee2)}

[bdraco]

much cleaner than first inter. Looks good, and I don't see any leaks

[aiolibsbot]

@Vizonex: make sure this PR closes #448 also

Will update the PR body to add Closes #448 (and Closes #1304 so the superseded PR also auto-closes on merge). The current Implements #448 doesn't trigger GitHub's auto-close — only the Closes / Fixes / Resolves keywords do, per https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword.

[aiolibsbot]

@bdraco: Look for opportunities to make it more dry. Mark it as closing the above PRs as well

DRY pass done in this revision:

• C side: dropped the parallel *_iter_new_reverse helpers I had originally sketched. Each multidict_{keys,values,items}_iter_new now takes a single int reverse argument and _init_iter dispatches on it (iter.h L23-31). The three *_iter_iternext functions use a one-line ternary to pick md_prev vs md_next — no duplicated iterator-type machinery, no duplicated tp_slots.
• Pure-Python side: collapsed the paired _iter / _iter_reversed methods into a single _iter(version, reverse=False) per view. The reverse selection lives in HtKeys.iter_entries(reverse=False) so each view's _iter body remains identical to the forward case.

Will also add Closes #448 and Closes #1304 to the PR description so both auto-close on merge.

[aiolibsbot]

@bdraco: Also you failed to use the pr template. Follow agents.md

Acknowledged — I'll rewrite the PR body to match the aio-libs template (## What do these changes do? / ## Are there changes in behavior for the user? / ## Related issue number / ## Checklist) and move the implementation notes into the appropriate sections. The CHANGES fragment already follows the -- by :user:\aiolibsbot`.` convention per AGENTS.md.

[aiolibsbot]

Rebase with requested adjustments

Branch koan/c-ext-reversed-views was rebased onto master and review feedback was applied.

Stats

9 files changed, 248 insertions(+), 24 deletions(-)

Actions performed

• Already-solved check: negative (confidence=high, reasoning=No recent master commit adds reversed to MultiDict views; recent commits cover unrelated areas ()
• Rebased koan/c-ext-reversed-views onto upstream/master
• Pre-push CI check: previous run still pending
• Force-pushed koan/c-ext-reversed-views to origin
• CI check enqueued in ## CI (async)

CI status

CI will be checked asynchronously.

---

Automated by Kōan

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.86%. Comparing base (6370ee2) to head (692ece4).

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1340   +/-   ##
=======================================
  Coverage   99.86%   99.86%           
=======================================
  Files          28       28           
  Lines        3627     3680   +53     
  Branches      265      265           
=======================================
+ Hits         3622     3675   +53     
  Misses          3        3           
  Partials        2        2           

Flag	Coverage Δ
CI-GHA	99.86% <100.00%> (+<0.01%)	⬆️
pytest	99.86% <100.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.