Skip to content

Commit 7ce1a1e

Browse files
knQzxmeta-codesync[bot]
authored andcommitted
detect slots layout conflict in multiple inheritance (#3011)
Summary: - added a check in class metadata computation that detects when two or more base classes define non-empty `__slots__`, which causes a TypeError at runtime in cpython - removed the `bug` markers from the existing test cases for #2916 and #2917 and added expected error comments - added new test cases for edge cases - empty slots, both empty, and three bases Pull Request resolved: #3011 Test Plan: - ran `cargo test -p pyrefly -- test_slots` - all 27 tests pass - tested the conflict detection for same slot names (#2916), different slot names (#2917), empty slots (no false positive), and three-base inheritance fixes #2916 fixes #2917 Reviewed By: yangdanny97 Differential Revision: D99692624 Pulled By: stroxler fbshipit-source-id: fe63b141eaabf1854a931bedae2b4d05aca13370
1 parent 2046e72 commit 7ce1a1e

2 files changed

Lines changed: 75 additions & 4 deletions

File tree

pyrefly/lib/alt/class/class_metadata.rs

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -402,6 +402,35 @@ impl<'a, Ans: LookupAnswer> AnswersSolver<'a, Ans> {
402402
}
403403
let extends_abc = self.extends_abc(&bases_with_metadata, metaclass);
404404

405+
// Check for __slots__ layout conflict: if two or more base classes
406+
// define non-empty __slots__, CPython raises TypeError at runtime
407+
// ("multiple bases have instance lay-out conflict").
408+
{
409+
let bases_with_nonempty_slots: Vec<&Class> = bases_with_metadata
410+
.iter()
411+
.filter(|(_, metadata)| {
412+
metadata.slots_info().is_some_and(|si| !si.names.is_empty())
413+
})
414+
.map(|(base, _)| base)
415+
.collect();
416+
if bases_with_nonempty_slots.len() > 1 {
417+
let names: Vec<String> = bases_with_nonempty_slots
418+
.iter()
419+
.map(|b| format!("`{}`", b.name()))
420+
.collect();
421+
self.error(
422+
errors,
423+
cls.range(),
424+
ErrorInfo::Kind(ErrorKind::InvalidInheritance),
425+
format!(
426+
"Class `{}` has multiple base classes with non-empty `__slots__` ({}), which causes a TypeError at runtime",
427+
cls.name(),
428+
names.join(", "),
429+
),
430+
);
431+
}
432+
}
433+
405434
// Compute final base class list.
406435
let bases = if is_typed_dict && bases_with_metadata.is_empty() {
407436
// This is a "fallback" class that contains attributes that are available on all TypedDict subclasses.

pyrefly/lib/test/slots.rs

Lines changed: 46 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,6 @@ c.x = 2 # OK: descriptor __set__ handles this, not instance storage
281281

282282
// https://github.com/facebook/pyrefly/issues/2917
283283
testcase!(
284-
bug = "Should detect instance layout conflict when multiple bases have __slots__",
285284
test_slots_multiple_inheritance_layout_conflict,
286285
r#"
287286
class Left:
@@ -292,13 +291,12 @@ class Right:
292291
293292
# Inheriting from two classes that both define non-empty __slots__
294293
# causes a TypeError at runtime.
295-
class Combined(Left, Right): ...
294+
class Combined(Left, Right): ... # E: multiple base classes with non-empty `__slots__`
296295
"#,
297296
);
298297

299298
// https://github.com/facebook/pyrefly/issues/2916
300299
testcase!(
301-
bug = "Should detect instance layout conflict even with identical slot names",
302300
test_slots_layout_conflict_same_names,
303301
r#"
304302
class First:
@@ -308,7 +306,51 @@ class Second:
308306
__slots__ = ("x",)
309307
310308
# Even though the slot names match, these are different C-level layouts.
311-
class Both(First, Second): ...
309+
class Both(First, Second): ... # E: multiple base classes with non-empty `__slots__`
310+
"#,
311+
);
312+
313+
testcase!(
314+
test_slots_layout_conflict_empty_slots_ok,
315+
r#"
316+
class A:
317+
__slots__ = ("x",)
318+
319+
class B:
320+
__slots__ = ()
321+
322+
# One base has non-empty slots, the other has empty slots - this is fine.
323+
class C(A, B): ...
324+
"#,
325+
);
326+
327+
testcase!(
328+
test_slots_layout_conflict_both_empty_ok,
329+
r#"
330+
class A:
331+
__slots__ = ()
332+
333+
class B:
334+
__slots__ = ()
335+
336+
# Both bases have empty slots - no conflict.
337+
class C(A, B): ...
338+
"#,
339+
);
340+
341+
testcase!(
342+
test_slots_layout_conflict_three_bases,
343+
r#"
344+
class A:
345+
__slots__ = ("x",)
346+
347+
class B:
348+
__slots__ = ("y",)
349+
350+
class C:
351+
__slots__ = ("z",)
352+
353+
class D(A, B, C): ... # E: multiple base classes with non-empty `__slots__`
312354
"#,
313355
);
314356

0 commit comments

Comments
 (0)