Skip to content

PyTypeFlags::{SEQUENCE,MAPPING} #6109

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
youknowone merged 1 commit into from
Aug 26, 2025
Merged

PyTypeFlags::{SEQUENCE,MAPPING} #6109

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion vm/src/builtins/dict.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ impl PyDict {
AsMapping,
Representable
),
flags(BASETYPE)
flags(BASETYPE, MAPPING)
)]
impl PyDict {
#[pyclassmethod]
Expand Down
2 changes: 1 addition & 1 deletion vm/src/builtins/list.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ pub type PyListRef = PyRef<PyList>;
AsSequence,
Representable
),
flags(BASETYPE)
flags(BASETYPE, SEQUENCE)
)]
impl PyList {
#[pymethod]
Expand Down
25 changes: 14 additions & 11 deletions vm/src/builtins/memory.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -538,17 +538,20 @@ impl Py<PyMemoryView> {
}
}

#[pyclass(with(
Py,
Hashable,
Comparable,
AsBuffer,
AsMapping,
AsSequence,
Constructor,
Iterable,
Representable
))]
#[pyclass(
with(
Py,
Hashable,
Comparable,
AsBuffer,
AsMapping,
AsSequence,
Constructor,
Iterable,
Representable
),
flags(SEQUENCE)
)]
impl PyMemoryView {
// TODO: Uncomment when Python adds __class_getitem__ to memoryview
// #[pyclassmethod]
Expand Down
21 changes: 12 additions & 9 deletions vm/src/builtins/range.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,15 +174,18 @@ pub fn init(context: &Context) {
PyRangeIterator::extend_class(context, context.types.range_iterator_type);
}

#[pyclass(with(
Py,
AsMapping,
AsSequence,
Hashable,
Comparable,
Iterable,
Representable
))]
#[pyclass(
with(
Py,
AsMapping,
AsSequence,
Hashable,
Comparable,
Iterable,
Representable
),
flags(SEQUENCE)
)]
impl PyRange {
fn new(cls: PyTypeRef, stop: ArgIndex, vm: &VirtualMachine) -> PyResult<PyRef<Self>> {
Self {
Expand Down
2 changes: 1 addition & 1 deletion vm/src/builtins/tuple.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ impl<T> PyTuple<PyRef<T>> {
}

#[pyclass(
flags(BASETYPE),
flags(BASETYPE, SEQUENCE),
with(
AsMapping,
AsSequence,
Expand Down
64 changes: 64 additions & 0 deletions vm/src/builtins/type.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ use crate::{
};
use indexmap::{IndexMap, map::Entry};
use itertools::Itertools;
use num_traits::ToPrimitive;
use std::{borrow::Borrow, collections::HashSet, ops::Deref, pin::Pin, ptr::NonNull};

#[pyclass(module = false, name = "type", traverse = "manual")]
Expand Down Expand Up @@ -231,6 +232,58 @@ impl PyType {
linearise_mro(mros)
}

/// Inherit SEQUENCE and MAPPING flags from base class (CPython: inherit_patma_flags)
fn inherit_patma_flags(slots: &mut PyTypeSlots, base: &PyRef<Self>) {
const COLLECTION_FLAGS: PyTypeFlags = PyTypeFlags::from_bits_truncate(
PyTypeFlags::SEQUENCE.bits() | PyTypeFlags::MAPPING.bits(),
);
if !slots.flags.intersects(COLLECTION_FLAGS) {
slots.flags |= base.slots.flags & COLLECTION_FLAGS;
}
}

/// Check for __abc_tpflags__ and set the appropriate flags
/// This checks in attrs and all base classes for __abc_tpflags__
fn check_abc_tpflags(
slots: &mut PyTypeSlots,
attrs: &PyAttributes,
bases: &[PyRef<Self>],
ctx: &Context,
) {
const COLLECTION_FLAGS: PyTypeFlags = PyTypeFlags::from_bits_truncate(
PyTypeFlags::SEQUENCE.bits() | PyTypeFlags::MAPPING.bits(),
);

// Don't override if flags are already set
if slots.flags.intersects(COLLECTION_FLAGS) {
return;
}

// First check in our own attributes
let abc_tpflags_name = ctx.intern_str("__abc_tpflags__");
if let Some(abc_tpflags_obj) = attrs.get(abc_tpflags_name) {
if let Some(int_obj) = abc_tpflags_obj.downcast_ref::<crate::builtins::int::PyInt>() {
let flags_val = int_obj.as_bigint().to_i64().unwrap_or(0);
let abc_flags = PyTypeFlags::from_bits_truncate(flags_val as u64);
slots.flags |= abc_flags & COLLECTION_FLAGS;
return;
}
}

// Then check in base classes
for base in bases {
if let Some(abc_tpflags_obj) = base.find_name_in_mro(abc_tpflags_name) {
if let Some(int_obj) = abc_tpflags_obj.downcast_ref::<crate::builtins::int::PyInt>()
{
let flags_val = int_obj.as_bigint().to_i64().unwrap_or(0);
let abc_flags = PyTypeFlags::from_bits_truncate(flags_val as u64);
slots.flags |= abc_flags & COLLECTION_FLAGS;
return;
}
}
}
}

Comment on lines +245 to +286
Copy link
Contributor

@coderabbitai coderabbitai bot Aug 25, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Fix: avoid silently zeroing or truncating abc_tpflags when converting from PyInt

Using to_i64().unwrap_or(0) will:

  • turn any large positive flag value (> i64::MAX) into 0,
  • turn any negative value into 0,
  • silently mask real issues.

At minimum, prefer an unsigned conversion and ignore non-convertible values; this avoids introducing incorrect zero flags. If you want CPython-compatible behavior (PyLong_AsUnsignedLongMask), you can additionally mask to the low 64 bits, but even the minimal fix below prevents accidental zeroing.

Apply this diff (both occurrences inside check_abc_tpflags):

-            if let Some(int_obj) = abc_tpflags_obj.downcast_ref::<crate::builtins::int::PyInt>() {
-                let flags_val = int_obj.as_bigint().to_i64().unwrap_or(0);
-                let abc_flags = PyTypeFlags::from_bits_truncate(flags_val as u64);
-                slots.flags |= abc_flags & COLLECTION_FLAGS;
-                return;
-            }
+            if let Some(int_obj) = abc_tpflags_obj.downcast_ref::<crate::builtins::int::PyInt>() {
+                if let Some(flags_val) = int_obj.as_bigint().to_u64() {
+                    let abc_flags = PyTypeFlags::from_bits_truncate(flags_val);
+                    slots.flags |= abc_flags & COLLECTION_FLAGS;
+                    return;
+                }
+            }

And:

-                if let Some(int_obj) = abc_tpflags_obj.downcast_ref::<crate::builtins::int::PyInt>()
-                {
-                    let flags_val = int_obj.as_bigint().to_i64().unwrap_or(0);
-                    let abc_flags = PyTypeFlags::from_bits_truncate(flags_val as u64);
-                    slots.flags |= abc_flags & COLLECTION_FLAGS;
-                    return;
-                }
+                if let Some(int_obj) = abc_tpflags_obj.downcast_ref::<crate::builtins::int::PyInt>()
+                {
+                    if let Some(flags_val) = int_obj.as_bigint().to_u64() {
+                        let abc_flags = PyTypeFlags::from_bits_truncate(flags_val);
+                        slots.flags |= abc_flags & COLLECTION_FLAGS;
+                        return;
+                    }
+                }

If you’d like parity with PyLong_AsUnsignedLongMask, I can follow up with a small helper that extracts the low 64 bits (two’s complement) from BigInt.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/// Check for __abc_tpflags__ and set the appropriate flags
/// This checks in attrs and all base classes for __abc_tpflags__
fn check_abc_tpflags(
slots: &mut PyTypeSlots,
attrs: &PyAttributes,
bases: &[PyRef<Self>],
ctx: &Context,
) {
const COLLECTION_FLAGS: PyTypeFlags = PyTypeFlags::from_bits_truncate(
PyTypeFlags::SEQUENCE.bits() | PyTypeFlags::MAPPING.bits(),
);
// Don't override if flags are already set
if slots.flags.intersects(COLLECTION_FLAGS) {
return;
}
// First check in our own attributes
let abc_tpflags_name = ctx.intern_str("__abc_tpflags__");
if let Some(abc_tpflags_obj) = attrs.get(abc_tpflags_name) {
if let Some(int_obj) = abc_tpflags_obj.downcast_ref::<crate::builtins::int::PyInt>() {
let flags_val = int_obj.as_bigint().to_i64().unwrap_or(0);
let abc_flags = PyTypeFlags::from_bits_truncate(flags_val as u64);
slots.flags |= abc_flags & COLLECTION_FLAGS;
return;
}
}
// Then check in base classes
for base in bases {
if let Some(abc_tpflags_obj) = base.find_name_in_mro(abc_tpflags_name) {
if let Some(int_obj) = abc_tpflags_obj.downcast_ref::<crate::builtins::int::PyInt>()
{
let flags_val = int_obj.as_bigint().to_i64().unwrap_or(0);
let abc_flags = PyTypeFlags::from_bits_truncate(flags_val as u64);
slots.flags |= abc_flags & COLLECTION_FLAGS;
return;
}
}
}
}
/// Check for __abc_tpflags__ and set the appropriate flags
/// This checks in attrs and all base classes for __abc_tpflags__
fn check_abc_tpflags(
slots: &mut PyTypeSlots,
attrs: &PyAttributes,
bases: &[PyRef<Self>],
ctx: &Context,
) {
const COLLECTION_FLAGS: PyTypeFlags = PyTypeFlags::from_bits_truncate(
PyTypeFlags::SEQUENCE.bits() | PyTypeFlags::MAPPING.bits(),
);
// Don't override if flags are already set
if slots.flags.intersects(COLLECTION_FLAGS) {
return;
}
// First check in our own attributes
let abc_tpflags_name = ctx.intern_str("__abc_tpflags__");
if let Some(abc_tpflags_obj) = attrs.get(abc_tpflags_name) {
if let Some(int_obj) = abc_tpflags_obj.downcast_ref::<crate::builtins::int::PyInt>() {
if let Some(flags_val) = int_obj.as_bigint().to_u64() {
let abc_flags = PyTypeFlags::from_bits_truncate(flags_val);
slots.flags |= abc_flags & COLLECTION_FLAGS;
return;
}
}
}
// Then check in base classes
for base in bases {
if let Some(abc_tpflags_obj) = base.find_name_in_mro(abc_tpflags_name) {
if let Some(int_obj) = abc_tpflags_obj.downcast_ref::<crate::builtins::int::PyInt>() {
if let Some(flags_val) = int_obj.as_bigint().to_u64() {
let abc_flags = PyTypeFlags::from_bits_truncate(flags_val);
slots.flags |= abc_flags & COLLECTION_FLAGS;
return;
}
}
}
}
}
🤖 Prompt for AI Agents 
In vm/src/builtins/type.rs around lines 245–286, the code uses
to_i64().unwrap_or(0) when reading __abc_tpflags__, which can silently turn
large/negative BigInt values into 0; replace that conversion with an
unsigned-aware approach: call the BigInt -> u64 conversion (e.g.
ToPrimitive::to_u64 or an equivalent helper) and if it returns None, treat the
value as non-convertible and skip applying flags (do not default to 0); if you
need CPython parity instead, extract the low 64 bits (unsigned mask) from the
BigInt (two’s complement) and use that u64. Apply this change in both places
inside check_abc_tpflags so flags are not silently zeroed or truncated.

#[allow(clippy::too_many_arguments)]
fn new_heap_inner(
base: PyRef<Self>,
Expand All @@ -246,6 +299,13 @@ impl PyType {
if base.slots.flags.has_feature(PyTypeFlags::HAS_DICT) {
slots.flags |= PyTypeFlags::HAS_DICT
}

// Inherit SEQUENCE and MAPPING flags from base class
Self::inherit_patma_flags(&mut slots, &base);

// Check for __abc_tpflags__ from ABCMeta (for collections.abc.Sequence, Mapping, etc.)
Self::check_abc_tpflags(&mut slots, &attrs, &bases, ctx);

if slots.basicsize == 0 {
slots.basicsize = base.slots.basicsize;
}
Expand Down Expand Up @@ -297,6 +357,10 @@ impl PyType {
if base.slots.flags.has_feature(PyTypeFlags::HAS_DICT) {
slots.flags |= PyTypeFlags::HAS_DICT
}

// Inherit SEQUENCE and MAPPING flags from base class
Self::inherit_patma_flags(&mut slots, &base);

if slots.basicsize == 0 {
slots.basicsize = base.slots.basicsize;
}
Expand Down
2 changes: 2 additions & 0 deletions vm/src/types/slot.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,8 @@ bitflags! {
#[non_exhaustive]
pub struct PyTypeFlags: u64 {
const MANAGED_DICT = 1 << 4;
const SEQUENCE = 1 << 5;
const MAPPING = 1 << 6;
const IMMUTABLETYPE = 1 << 8;
const HEAPTYPE = 1 << 9;
const BASETYPE = 1 << 10;
Expand Down
Loading