Skip to content

Validate compile() filename type and dont_inherit __bool__ protocol#7768

Merged
youknowone merged 1 commit into
RustPython:mainfrom
changjoon-park:fix-compile-filename-dont-inherit
May 4, 2026
Merged

Validate compile() filename type and dont_inherit __bool__ protocol#7768
youknowone merged 1 commit into
RustPython:mainfrom
changjoon-park:fix-compile-filename-dont-inherit

Conversation

@changjoon-park
Copy link
Copy Markdown
Contributor

@changjoon-park changjoon-park commented May 3, 2026

Summary

Two CPython parity gaps in compile() argument handling.

1. filename — buffer-protocol types should be rejected

>>> compile('pass', bytearray(b'file.py'), 'exec')
# CPython 3.14.4: TypeError
# RustPython main: <code object>  ❌

CPython's PyUnicode_FSDecoder accepts str, bytes, and objects with __fspath__ only — bytearray / memoryview / array.array raise TypeError. RustPython's FsPath::TryFromObject impl falls back to any buffer-protocol object and silently converts to bytes.

Change CompileArgs::filename from FsPath (which uses the permissive TryFromObject impl) to PyObjectRef, then call the strict FsPath::try_from_path_like at the top of compile(). Other FsPath consumers are unchanged.

2. dont_inherit — should call __bool__, not strict-check the bool type

>>> class EvilBool:
...     def __bool__(self): raise ValueError('hi')
>>> compile('pass', 'f', 'exec', dont_inherit=EvilBool())
# CPython 3.14.4: ValueError('hi')
# RustPython main: TypeError('Expected type bool, not EvilBool')  ❌

CPython routes dont_inherit through PyObject_IsTrue, calling __bool__ on arbitrary objects and propagating any exception raised there. Switch from OptionalArg<bool> to OptionalArg<ArgIntoBool> (already used elsewhere in builtins for all, any, print(..., flush=...)).

Verification (CPython 3.14.4)

Probe Cases Match
filename basic types (str, bytes, bytearray, memoryview, list, int) 6 6/6
filename edge cases (NUL byte, pathlib.Path, custom __fspath__, bad __fspath__, str/bytes subclass) 8 8/8
dont_inherit basic values (True/False/1/0/""/"yes"/None) + EvilBool 8 8/8
dont_inherit edge cases (int, float, list, dict, non-bool __bool__, plain object) 6 6/6

Tests unmasked

Lib/test/test_compile.py:

  • test_compile_filename
  • test_compile_filename_refleak

Test runs

cargo run --release -- -m test test_compile test_builtin test_compileall test_codeop test_ast — 703 tests pass, 0 regressions. All 188 extra_tests/snippets/*.py pass under the CI feature set.

Summary by CodeRabbit

  • Bug Fixes
    • Improved compile() function for enhanced CPython compatibility with stricter parameter validation and better truthiness evaluation.

Two CPython parity gaps in `compile()` argument handling:

1. **filename rejects only non-buffer-protocol types.** CPython's
   `PyUnicode_FSDecoder` accepts `str`, `bytes`, and objects with
   `__fspath__` only — `bytearray` / `memoryview` / `array.array` raise
   TypeError. RustPython's `FsPath::TryFromObject` impl falls back to
   any buffer-protocol object and silently converts to bytes, accepting
   them.

   ```python
   >>> compile('pass', bytearray(b'file.py'), 'exec')
   # CPython 3.14.4: TypeError
   # RustPython main: <code object>  ❌
   ```

   Change `CompileArgs::filename` from `FsPath` (which uses the
   permissive `TryFromObject` impl) to `PyObjectRef`, then call the
   strict `FsPath::try_from_path_like` at the top of `compile()`. Other
   `FsPath` consumers are unchanged.

2. **dont_inherit strict-checks the `bool` type.** CPython routes
   `dont_inherit` through `PyObject_IsTrue`, calling `__bool__` on
   arbitrary objects and propagating exceptions raised there. RustPython
   typed it `OptionalArg<bool>`, which rejects subclass-less objects at
   binding time before `__bool__` runs.

   ```python
   >>> class EvilBool:
   ...     def __bool__(self): raise ValueError('hi')
   >>> compile('pass', 'f', 'exec', dont_inherit=EvilBool())
   # CPython 3.14.4: ValueError('hi')
   # RustPython main: TypeError('Expected type bool, not EvilBool')  ❌
   ```

   Switch to `OptionalArg<ArgIntoBool>` — already used elsewhere in
   `builtins` (`all`, `any`, `print(..., flush=...)`).

Verified byte-identical with CPython 3.14.4 across:
- 6 filename types (str, bytes, bytearray, memoryview, list, int)
- 7 dont_inherit values (True/False/1/0/""/"yes"/None) plus the
  `__bool__`-raises-ValueError case

Unmasks `Lib/test/test_compile.py`:
- `test_compile_filename`
- `test_compile_filename_refleak`

`cargo run --release -- -m test test_compile test_builtin test_compileall
test_codeop test_ast` — 703 tests pass, 0 regressions. All 188
`extra_tests/snippets/*.py` pass under the CI feature set.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 3, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 5885a4e4-d70d-4235-87ca-50c447a69809

📥 Commits

Reviewing files that changed from the base of the PR and between c2910c0 and 19931e1.

⛔ Files ignored due to path filters (1)
  • Lib/test/test_compile.py is excluded by !Lib/**
📒 Files selected for processing (1)
  • crates/vm/src/stdlib/builtins.rs

📝 Walkthrough

Walkthrough

The compile() builtin function's argument parsing is enhanced for CPython compatibility. The dont_inherit parameter now accepts any object with __bool__ conversion semantics, and the filename argument is validated upfront to reject buffer-protocol types while being normalized into a stricter path representation used consistently throughout the function.

Changes

Argument & Filename Handling

Layer / File(s) Summary
Argument Type
crates/vm/src/stdlib/builtins.rs (lines 100–114)
CompileArgs.dont_inherit changes from OptionalArg<bool> to OptionalArg<ArgIntoBool> to honor bool conversion via __bool__ and propagate conversion exceptions.
Filename Validation
crates/vm/src/stdlib/builtins.rs (lines 270–275)
args.filename is resolved upfront via FsPath::try_from_path_like(..., true, vm) to create a stricter filename value, rejecting buffer-protocol inputs like bytearray and memoryview.
Compile & Parse Integration
crates/vm/src/stdlib/builtins.rs (lines 336, 357, 404)
AST/bytecode compile, parser/decode, and compiler/bytecode paths now use the normalized filename (via filename.to_string_lossy() or .into_owned()) instead of the original args.filename for consistency and correctness.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • youknowone
  • ShaharNaveh

Poem

🐰 A rabbit hops through compile's gate,
Where filenames and flags await,
No buffers sneak, no bool's unsaid—
All paths are clear, all truthiness read!
CPython's way, now RustPython's too. ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and specifically describes the main changes: validating compile() filename type and implementing bool protocol support for dont_inherit.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share
Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 3, 2026

📦 Library Dependencies

The following Lib/ modules were modified. Here are their dependencies:

[x] test: cpython/Lib/test/test_compile.py (TODO: 16)
[x] test: cpython/Lib/test/test_compiler_assemble.py
[x] test: cpython/Lib/test/test_compiler_codegen.py
[x] test: cpython/Lib/test/test_peepholer.py (TODO: 10)

dependencies:

dependent tests: (no tests depend on compile)

Legend:

  • [+] path exists in CPython
  • [x] up-to-date, [ ] outdated

Copy link
Copy Markdown
Contributor

@ShaharNaveh ShaharNaveh left a comment

Choose a reason for hiding this comment

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

ty:)

@youknowone youknowone merged commit 90cc888 into RustPython:main May 4, 2026
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants