Skip to content

Commit e27697d

Browse files
fregataaclaude
fregataa
and
claude
committed
Narrow visibility of the new helpers and trim their docs
from_object() and ArgIterable::as_object() had no caller outside their own file and crate respectively, so drop them to private and pub(crate). The dict_inner helpers stay pub(crate) because builtins::set calls them. Also shorten the doc comments to match the density of the surrounding code; only insert_known_hash keeps a real note, since a wrong hash there corrupts the table silently. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 561d16b commit e27697d

3 files changed

Lines changed: 23 additions & 38 deletions

File tree

‎crates/vm/src/builtins/set.rs‎

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -195,20 +195,17 @@ impl PySetInner {
195195
Ok(set)
196196
}
197197

198-
/// Build a set from an arbitrary object, reusing the source's stored
199-
/// hashes when it is a set/frozenset/dict.
200-
pub(super) fn from_object(iterable: PyObjectRef, vm: &VirtualMachine) -> PyResult<Self> {
198+
/// Build a set from an arbitrary object, reusing stored hashes when the
199+
/// source is a set/frozenset/dict.
200+
fn from_object(iterable: PyObjectRef, vm: &VirtualMachine) -> PyResult<Self> {
201201
let set = Self::default();
202202
set.update_internal(iterable, vm)?;
203203
Ok(set)
204204
}
205205

206-
/// Elements of `obj` paired with the hash already stored alongside them,
207-
/// or `None` if `obj` keeps no such hashes and has to be iterated
208-
/// generically (which calls `__hash__` on every element again).
209-
///
210-
/// Mirrors the `PyAnySet_Check` / `PyDict_CheckExact` fast paths in
211-
/// CPython's `set_update_internal`.
206+
/// Elements of `obj` with their stored hashes, or `None` if `obj` keeps
207+
/// none and must be iterated generically. Mirrors the `PyAnySet_Check` /
208+
/// `PyDict_CheckExact` fast paths in CPython's `set_update_internal`.
212209
fn cached_hashes(obj: &PyObject, vm: &VirtualMachine) -> Option<Vec<(PyObjectRef, PyHash)>> {
213210
if let Some(set) = extract_set(obj) {
214211
Some(set.content.keys_with_hashes())
@@ -251,9 +248,8 @@ impl PySetInner {
251248
Self::wrap_unhashable_error(result, needle, vm)
252249
}
253250

254-
/// [`Self::contains`] for a needle whose hash was read from another
255-
/// set/dict. Such a needle is hashable by construction, so there is no
256-
/// set-to-frozenset retry to do.
251+
/// [`Self::contains`] with a known hash. Such a needle came out of a
252+
/// set/dict, so it is hashable and needs no frozenset retry.
257253
fn contains_known_hash(
258254
&self,
259255
needle: &PyObject,
@@ -398,7 +394,7 @@ impl PySetInner {
398394
Self::wrap_unhashable_error(result, &item, vm)
399395
}
400396

401-
/// [`Self::add`] for an item whose hash was read from another set/dict.
397+
/// [`Self::add`] with a known hash.
402398
fn add_known_hash(&self, item: PyObjectRef, hash: PyHash, vm: &VirtualMachine) -> PyResult<()> {
403399
let result = self.content.insert_known_hash(vm, &*item, hash, ());
404400
Self::wrap_unhashable_error(result, &item, vm)
@@ -1098,8 +1094,6 @@ impl Constructor for PyFrozenSet {
10981094
}
10991095

11001096
fn py_new(_cls: &Py<PyType>, iterable: Self::Args, vm: &VirtualMachine) -> PyResult<Self> {
1101-
// built from the object itself, so a set/frozenset/dict source can
1102-
// hand over its stored hashes instead of being re-hashed
11031097
let inner = match iterable {
11041098
OptionalArg::Present(iterable) => PySetInner::from_object(iterable, vm)?,
11051099
OptionalArg::Missing => PySetInner::default(),

‎crates/vm/src/dict_inner.rs‎

Lines changed: 13 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -468,11 +468,9 @@ impl<T: Clone> Dict<T> {
468468

469469
/// Store a key whose hash the caller already knows.
470470
///
471-
/// `hash` MUST be the value `key.key_hash(vm)` would return. Passing a
472-
/// different hash corrupts the table: the entry is stored in a bucket no
473-
/// lookup will probe, so the key silently goes missing. Only pass a hash
474-
/// read out of another dict/set entry holding this very key object, e.g.
475-
/// via [`Self::keys_with_hashes`].
471+
/// `hash` must equal `key.key_hash(vm)`; a wrong one lands the entry in a
472+
/// bucket no lookup probes, silently losing the key. Only pass a hash from
473+
/// [`Self::keys_with_hashes`] on a container holding this same key.
476474
pub(crate) fn insert_known_hash<K>(
477475
&self,
478476
vm: &VirtualMachine,
@@ -535,10 +533,8 @@ impl<T: Clone> Dict<T> {
535533
self.contains_known_hash(vm, key, key_hash)
536534
}
537535

538-
/// [`Self::contains`] with a caller-supplied hash.
539-
///
540-
/// Same contract as [`Self::insert_known_hash`]: a wrong `hash` makes the
541-
/// lookup probe the wrong bucket and report a present key as missing.
536+
/// [`Self::contains`] with a known hash. Same contract as
537+
/// [`Self::insert_known_hash`].
542538
pub(crate) fn contains_known_hash<K: DictKey + ?Sized>(
543539
&self,
544540
vm: &VirtualMachine,
@@ -730,9 +726,8 @@ impl<T: Clone> Dict<T> {
730726
self.remove_if_exists(vm, key).map(|opt| opt.is_some())
731727
}
732728

733-
/// [`Self::delete_if_exists`] with a caller-supplied hash.
734-
///
735-
/// Same contract as [`Self::insert_known_hash`].
729+
/// [`Self::delete_if_exists`] with a known hash. Same contract as
730+
/// [`Self::insert_known_hash`].
736731
pub(crate) fn delete_if_exists_known_hash<K>(
737732
&self,
738733
vm: &VirtualMachine,
@@ -777,9 +772,8 @@ impl<T: Clone> Dict<T> {
777772
self.remove_if_known_hash(vm, key, hash, pred)
778773
}
779774

780-
/// [`Self::remove_if`] with a caller-supplied hash.
781-
///
782-
/// Same contract as [`Self::insert_known_hash`].
775+
/// [`Self::remove_if`] with a known hash. Same contract as
776+
/// [`Self::insert_known_hash`].
783777
fn remove_if_known_hash<K, F>(
784778
&self,
785779
vm: &VirtualMachine,
@@ -811,9 +805,8 @@ impl<T: Clone> Dict<T> {
811805
self.delete_or_insert_known_hash(vm, key, hash, value)
812806
}
813807

814-
/// [`Self::delete_or_insert`] with a caller-supplied hash.
815-
///
816-
/// Same contract as [`Self::insert_known_hash`].
808+
/// [`Self::delete_or_insert`] with a known hash. Same contract as
809+
/// [`Self::insert_known_hash`].
817810
pub(crate) fn delete_or_insert_known_hash(
818811
&self,
819812
vm: &VirtualMachine,
@@ -967,10 +960,8 @@ impl<T: Clone> Dict<T> {
967960
.collect()
968961
}
969962

970-
/// All keys paired with the hash already stored in their entry.
971-
///
972-
/// Lets a caller move keys into another dict/set without calling
973-
/// `__hash__` again; see [`Self::insert_known_hash`].
963+
/// All keys paired with the hash stored in their entry, for feeding
964+
/// [`Self::insert_known_hash`] without re-calling `__hash__`.
974965
pub(crate) fn keys_with_hashes(&self) -> Vec<(PyObjectRef, HashValue)> {
975966
self.read()
976967
.entries

‎crates/vm/src/function/protocol.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ unsafe impl<T: Traverse> Traverse for ArgIterable<T> {
8787

8888
impl<T> ArgIterable<T> {
8989
#[must_use]
90-
pub fn as_object(&self) -> &PyObject {
90+
pub(crate) fn as_object(&self) -> &PyObject {
9191
&self.iterable
9292
}
9393

0 commit comments

Comments
 (0)