Accept __index__-conforming objects for compile() flags / optimize#7728
Conversation
CPython's compile() (Python/Python-ast.c) accepts any object with __index__ for the flags and optimize arguments. RustPython's CompileArgs typed both fields as OptionalArg<PyIntRef>, so a class with only __index__ raised 'TypeError: Expected type int but X found' during arg binding. bpo-36907's regression test (test_call.test_fastcall_clearing_dict) exercises exactly this: an IntWithDict.__index__ that mutates self.kwargs mid-call. CPython parses both args via __index__ and doesn't crash even when the kwargs dict is cleared between binding and use. Switch flags and optimize to OptionalArg<ArgPrimitiveIndex<i32>>, the same helper already used by range, slice, bytes.__mul__, hex, oct, etc. ArgPrimitiveIndex calls try_index (= __index__ protocol) and converts to the requested primitive in one step, so the three call sites in compile() simplify from .map_or(Ok(d), |v| v.try_to_primitive(vm))? to .map_or(d, |v| v.value). Unmasks test_call.FastCallTests.test_fastcall_clearing_dict.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
📦 Library DependenciesThe following Lib/ modules were modified. Here are their dependencies: [ ] test: cpython/Lib/test/test_call.py dependencies: dependent tests: (no tests depend on call) Legend:
|
Background
CPython's
compile()(Python/Python-ast.c) accepts any object with__index__for theflagsandoptimizearguments — not just exactint. RustPython'sCompileArgstyped both fields asOptionalArg<PyIntRef>, so a class with only__index__raisedTypeError: Expected type 'int' but '<X>' found.during arg binding.bpo-36907's regression test (
test_call.test_fastcall_clearing_dict) exercises exactly this: anIntWithDict.__index__that mutatesself.kwargsmid-call. CPython parses both args via__index__, doesn't crash even when the kwargs dict is cleared between binding and use.Repro
Fix
Change
flagsandoptimizeinCompileArgsfromOptionalArg<PyIntRef>toOptionalArg<ArgPrimitiveIndex<i32>>.ArgPrimitiveIndex(already used byrange,slice,bytes.__mul__,hex,oct, ...) calls__index__then converts to the primitive in one step. Three call sites simplify from.map_or(Ok(default), |v| v.try_to_primitive(vm))?to.map_or(default, |v| v.value).Tests unmasked
test_call.FastCallTests.test_fastcall_clearing_dictVerification
compile("...", "<t>", "eval", 0),compile(..., flags=0),compile(..., optimize=1)__index__objects still raiseTypeError(now fromtry_indexinstead of strictPyIntRefcheck)test_call,test_compile,test_int,test_index,test_descr,test_typing,test_ast(~1,571 tests)test_termios.pyhas 4 instances ofExpected type 'int' but 'FileIO' found., but those are a different fix path (file-descriptor protocol viafileno(), not__index__); out of scope.Summary by CodeRabbit
compile()function's argument handling to accept a broader range of numeric types, enhancing compatibility with Python conventions.