nt.skiproot, winapi.LCMapStringEx #6399

youknowone · 2025-12-11T11:22:44Z

Summary by CodeRabbit

New Features
- Added Windows path component splitting capability for enhanced path handling.
- Added Windows string mapping functionality for locale-aware string operations.
- Added Windows named pipe creation support with configurable parameters.
- Added Windows locale constant for invariant culture support.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

coderabbitai · 2025-12-11T11:22:50Z

Walkthrough

This pull request adds Windows-specific functionality to the Rust VM: a CPython-like path splitting utility that decomposes Windows paths into drive, root, and tail components while preserving input type (str or bytes), and Windows API bindings for string locale mapping and named pipe creation with UTF-16 encoding/decoding support.

Changes

Cohort / File(s)	Summary
Windows path utilities `crates/vm/src/stdlib/nt.rs`	Added `_path_splitroot_ex()` function that accepts path-like inputs (str or bytes), normalizes slashes, computes drive and root segment lengths via `skiproot()` helper, and returns a 3-tuple of (drive, root, tail) while preserving input type and handling WTF-8 surrogates.
Windows API bindings `crates/vm/src/stdlib/winapi.rs`	Added `LOCALE_NAME_INVARIANT` constant, `LCMapStringEx()` function for locale-aware string case mapping with Windows API integration and encoding validation, and `CreateNamedPipe()` function for creating named pipes with structured argument handling.

Estimated code review effort

🎯 3 (Moderate) | ��️ ~25 minutes

Specific areas for attention:
- WTF-8 surrogate handling and UTF-16 round-trip encoding/decoding logic in both files
- Windows path semantics in _path_splitroot_ex() including drive and UNC root detection
- Error propagation and type validation across str/bytes input variants in nt.rs
- Windows API call semantics and buffer handling in LCMapStringEx() (two-phase sizing pattern)
- Argument struct CreateNamedPipeArgs field mapping to Windows CreateNamedPipeW() parameters

Poem

🐰 With paths now split on Windows wide,
And locale maps that shall not hide,
Named pipes dance through systems grand—
The VM grows stronger, isn't that grand!
UTF-16 surrogates handled with care,
New functions blooming everywhere! 🌟

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title 'nt.skiproot, winapi.LCMapStringEx' directly references the two main functions being added, but lacks clarity about the purpose and nature of these additions.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ 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.}

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (1)

crates/vm/src/stdlib/winapi.rs (1)
646-671: Consider using WTF-8 encoding for consistency.

The LCMapStringEx function (line 570-574) uses as_wtf8().encode_wide() for proper surrogate handling, while CreateNamedPipe uses as_str().to_wide_with_nul(). For consistency and to properly handle strings containing surrogates, consider using the same pattern:
-        let name_wide = args.name.as_str().to_wide_with_nul();
+        let name_wide: Vec<u16> = args.name
+            .as_wtf8()
+            .encode_wide()
+            .chain(std::iter::once(0))
+            .collect();
While pipe names are typically ASCII paths like \\.\pipe\name, using the WTF-8 aware encoding maintains consistency across the module.

📜 Review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4828fb3 and 608c98c.

⛔ Files ignored due to path filters (3)

Lib/test/test_fnmatch.py is excluded by !Lib/**
Lib/test/test_ntpath.py is excluded by !Lib/**
Lib/test/test_posixpath.py is excluded by !Lib/**

📒 Files selected for processing (2)

crates/vm/src/stdlib/nt.rs (1 hunks)
crates/vm/src/stdlib/winapi.rs (1 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

**/*.rs

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.rs: Follow the default rustfmt code style by running cargo fmt to format Rust code
Always run clippy to lint Rust code (cargo clippy) before completing tasks and fix any warnings or lints introduced by changes
Follow Rust best practices for error handling and memory management
Use the macro system (pyclass, pymodule, pyfunction, etc.) when implementing Python functionality in Rust

Files:

crates/vm/src/stdlib/nt.rs
crates/vm/src/stdlib/winapi.rs

🧬 Code graph analysis (1)

crates/vm/src/stdlib/winapi.rs (1)

crates/wtf8/src/lib.rs (1)

from_wide (336-352)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)

GitHub Check: Ensure compilation on various targets
GitHub Check: Run rust tests (windows-latest)
GitHub Check: Run snippets and cpython tests (windows-latest)
GitHub Check: Run snippets and cpython tests (macos-latest)
GitHub Check: Run snippets and cpython tests (ubuntu-latest)
GitHub Check: Check Rust code with clippy

🔇 Additional comments (5)

crates/vm/src/stdlib/nt.rs (2)

573-643: LGTM! Well-implemented Windows path root detection.

The skiproot function correctly implements CPython's _Py_skiproot logic, properly handling UNC paths, device paths (\\?\UNC\), drive letters, and relative paths. The safe get closure pattern avoids bounds checking issues.

645-716: Well-structured implementation with proper type preservation.

The function correctly:

Handles path-like objects via __fspath__

Uses WTF-8 encoding for PyStr to handle surrogates

Validates UTF-8 for bytes input with appropriate error messages

Preserves original separators by using wide (not normalized) for reconstruction

Returns the correct output type (bytes or str) matching the input type

crates/vm/src/stdlib/winapi.rs (3)

543-546: LGTM!

Correctly defines LOCALE_NAME_INVARIANT as an empty string, matching the Windows API specification.

548-624: Well-implemented locale string mapping with proper WTF-8 handling.

The implementation correctly:

Validates unsupported flags (matching CPython behavior)

Uses WTF-8 encoding via as_wtf8().encode_wide() for proper surrogate handling

Implements the standard two-phase Windows API pattern for buffer sizing

Checks for i32::MAX overflow on input length

Uses Wtf8Buf::from_wide for output which correctly handles unpaired surrogates

626-644: LGTM!

The struct correctly defines all parameters for CreateNamedPipeW. The ignored _security_attributes is appropriately documented.

nt.skiproot, winapi.LCMapStringEx

608c98c

youknowone force-pushed the ntpath branch from dd459ed to 608c98c Compare December 11, 2025 11:54

youknowone marked this pull request as ready for review December 11, 2025 12:01

coderabbitai bot reviewed Dec 11, 2025

View reviewed changes

youknowone merged commit 0054394 into RustPython:main Dec 11, 2025
13 checks passed

youknowone deleted the ntpath branch December 11, 2025 12:42

coderabbitai bot mentioned this pull request Dec 11, 2025

nt junction #6407

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

nt.skiproot, winapi.LCMapStringEx #6399

nt.skiproot, winapi.LCMapStringEx #6399

Uh oh!

youknowone commented Dec 11, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Dec 11, 2025 •

edited

Loading

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

nt.skiproot, winapi.LCMapStringEx #6399

nt.skiproot, winapi.LCMapStringEx #6399

Uh oh!

Conversation

youknowone commented Dec 11, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

Pre-merge checks and finishing touches

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

youknowone commented Dec 11, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Dec 11, 2025 •

edited

Loading