Skip to content

Support #[cfg] in pymodule with#6975

Merged
youknowone merged 1 commit intoRustPython:mainfrom
youknowone:cfg-with
Feb 3, 2026
Merged

Support #[cfg] in pymodule with#6975
youknowone merged 1 commit intoRustPython:mainfrom
youknowone:cfg-with

Conversation

@youknowone
Copy link
Member

@youknowone youknowone commented Feb 3, 2026 •
edited by coderabbitai bot
Loading

Summary by CodeRabbit

  • New Features

    • Enhanced attribute parsing to support cfg-aware module items, enabling conditional inclusion of submodules.

  • Refactor

    • Reworked module initialization and metadata assembly to honor cfg-gated with(...) entries across modules.

  • Bug Fixes

    • Corrected platform gating for stdlib modules (ssl, time), removing unconditional stubs and fixing platform-specific exports.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 3, 2026 •
edited
Loading

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

Refactors pymodule attribute parsing: introduces PyModuleArgs and WithItem, makes pymodule/impl_pymodule accept parsed args instead of PunctuatedNestedMeta, adds cfg-aware handling for with(...) items, removes ModuleItemMeta::with(), and updates stdlib usages to use cfg-gated with items. (48 words)

Changes

Cohort / File(s) Summary
Derive-impl public API & entry
crates/derive-impl/src/lib.rs		 Re-exported PyModuleArgs and changed pymodule signature to accept PyModuleArgs.
Pymodule parsing & generation
crates/derive-impl/src/pymodule.rs		 Added WithItem and PyModuleArgs types and parsing; switched impl to accept PyModuleArgs; implemented cfg-aware partitioning of with-items, const gating, negated cfg helpers, and adjusted METHOD_DEFS / __init_attributes generation.
Utility parsing rules
crates/derive-impl/src/util.rs		 Removed ModuleItemMeta::with() and updated ALLOWED_NAMES to exclude "with".
Derive wrapper
crates/derive/src/lib.rs		 Now parses pymodule attributes as derive_impl::PyModuleArgs before calling into derive_impl.
Stdlib module wiring
crates/stdlib/src/openssl.rs, crates/vm/src/stdlib/time.rs		 Rewrote with(...) lists to include #[cfg(...)]-gated items and removed unconditional empty submodule stubs / wasm32 fallback stubs.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • coolreader18
  • ShaharNaveh

Poem

🐰
I parsed the args with nimble paws,
WithItem hops through cfg-gated laws,
No stub left standing in my wake,
Module lists now only compile what’s awake,
Hooray — a hop, a patch, and cake! 🎉

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: adding support for conditional compilation attributes (#[cfg]) in pymodule with(...) expressions.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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

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

@youknowone youknowone force-pushed the cfg-with branch 2 times, most recently from b221086 to 306d939 Compare February 3, 2026 08:57
@youknowone youknowone marked this pull request as ready for review February 3, 2026 09:39
coderabbitai[bot]
coderabbitai bot reviewed Feb 3, 2026
View reviewed changes
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents 
In `@crates/derive-impl/src/pymodule.rs`:
- Around line 45-80: The parser for PyModuleArgs currently assigns to with_items
when it encounters a with(...) block, so later with(...) blocks overwrite
earlier ones; in the impl syn::parse::Parse for PyModuleArgs (the parse
function) change the handling inside the "if ident == \"with\"" branch to merge
parsed items into the existing with_items (use Vec::extend or similar on the
Vec<WithItem>) instead of replacing it, ensuring you still parse the trailing
comma and continue; reference the with_items local variable and the parse of
syn::punctuated::Punctuated::<WithItem,...>::parse_terminated(...) as the
locations to modify.

@youknowone youknowone merged commit 477e20a into RustPython:main Feb 3, 2026
@youknowone youknowone deleted the cfg-with branch February 3, 2026 09:56
@coderabbitai coderabbitai bot mentioned this pull request Feb 13, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@coderabbitai coderabbitai[bot] coderabbitai[bot] left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@youknowone