address review: assignment scan and wasm TLS by youknowone · Pull Request #8716 · RustPython/RustPython

youknowone · 2026-09-14T14:54:48Z

Follow-up to #8701 review comments that landed after the squash merge.

Skip # comments and lambda defaults in condition_plain_assignment
Restrict the suite-header colon fallback to yield targets
Capture __del__ before calling it so the unraisable names the destructor that ran
Store browser _socket timeouts as f64 bits and accept integers via ArgIntoFloat
Fail closed when wasm verify_mode != CERT_NONE; export getpeercert; feed MemoryBIO EOF into TLS

Assisted-by: Grok:grok-4.6

Summary by CodeRabbit

Bug Fixes
- Improved parsing of assignment targets and conditions involving suite headers, comments, and lambda expressions.
- Corrected object cleanup behavior so finalizers are resolved consistently.
- Fixed TLS connection handling, including EOF propagation and certificate verification when no trusted CA store is available.
API Changes
- Socket timeouts now preserve fractional-second values and reject NaN or non-finite values.
- SSL peer certificate retrieval now uses getpeercert, returning DER data or an empty result after handshake completion.

Skip comments and lambda defaults in the condition assignment scanner. Restrict the suite-header colon fallback to yield targets. Capture __del__ before calling it. Store browser socket timeouts as f64 and accept integers. Fail closed when wasm verify_mode is not CERT_NONE. Export getpeercert and feed MemoryBIO EOF. Assisted-by: Grok:grok-4.6

chatgpt-codex-connector · 2026-09-14T14:55:08Z

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-14T16:41:22.956573Z	`f7e2a34`	New commits
🔒 Security Review	✅ Completed 2026-09-14T16:45:58.916549Z	`f7e2a34`	New commits

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

coderabbitai · 2026-09-14T14:55:09Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Advanced

Run ID: 6237a750-1bb1-4ae2-8862-acf1d5fff9f9

📥 Commits

Reviewing files that changed from the base of the PR and between 73144c4 and f7e2a34.

📒 Files selected for processing (1)

crates/compiler/src/lib.rs

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.

📝 Walkthrough

Walkthrough

The pull request updates compiler scanners, object finalization lookup, WebAssembly socket timeout handling, and WebAssembly SSL behavior.

Changes

Compiler scanner updates

Layer / File(s)	Summary
Assignment and condition scanner handling `crates/compiler/src/lib.rs`	Assignment-target scanning recognizes parenthesized `yield` expressions after a top-level colon. Condition scanning skips comments and ignores plain `=` inside lambda parameter contexts.

Object finalization lookup

Layer / File(s)	Summary
Deallocator method lookup `crates/vm/src/object/core.rs`	`call_slot_del` retrieves `__del__` before calling `slot_del`. Existing error handling remains in place.

Socket timeout handling

Layer / File(s)	Summary
Floating-point timeout storage and validation `crates/wasm/src/socket.rs`	Default and per-socket timeouts preserve `f64` values. Timeout arguments use `ArgIntoFloat`, and NaN or non-finite values raise validation errors.

SSL connection behavior

Layer / File(s)	Summary
SSL connection and certificate behavior `crates/wasm/src/ssl.rs`	Client setup reports certificate verification failure when verification is enabled without a trusted store. BIO EOF is propagated, and `getpeercert` replaces `peer_certificate` with handshake and certificate handling.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Suggested reviewers: name-of-okja

Merge Risk: ⚪ Minimal · up to f7e2a

The compiler, socket, destructor, and WebAssembly SSL changes show no concrete merge-blocking regression.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 43.75% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 16 functions across 4 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title identifies two major changes: assignment scanning and wasm TLS. It is concise and related to the pull request, although it does not mention the socket timeout and destructor updates.
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.

Fix all pre-merge checks with AI

✨ 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

_{Comment @coderabbitai help to get the list of available commands.}

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 73144c4a75

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-09-14T15:00:53Z

    let colon = top_level_colon(bytes, target_start, target_end)?;
    let after = skip_horizontal_whitespace(bytes, colon + 1);
-    if after >= target_end {
+    if after >= target_end || !starts_identifier(bytes, after, b"yield") {


Allow parentheses before the post-header yield

For the documented input def f(): (yield bar) = y, after points to ( rather than yield, so this guard returns None and disables the suite-header fallback it is intended to preserve. Consequently, the existing parenthesized_yield_assignment_uses_invalid_target_message and parenthesized_yield_augassign_uses_illegal_expression_message tests receive a generic parser diagnostic instead of their required CPython-compatible messages; inspect or parse the post-colon expression through its parentheses before rejecting it.

Useful? React with 👍 / 👎.

Applied. The suite-header fallback still starts at the first token after :, but the yield check now walks through grouping parentheses, so def f(): (yield bar) = y keeps the invalid-target messages.
— commented by Claude

Walk through grouping parentheses before requiring yield after a suite-header colon. Assisted-by: Grok:grok-4.6

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f7e2a3429a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

chatgpt-codex-connector · 2026-09-14T16:41:20Z

+            } else if eof {
+                conn.feed_tls(&[]).map_err(|err| map_tls(vm, err))?;


Drain buffered plaintext before reporting BIO EOF

When the incoming MemoryBIO has reached EOF, every call to pump() now feeds an empty slice, and TlsConnection::feed_tls immediately returns ZeroReturn or Eof. If the final TLS records produced more plaintext than the previous read(len) consumed—for example, the peer sent a response and closed while the caller reads it in small chunks—the next read() fails here before consulting rustls's buffered plaintext, making the remainder permanently inaccessible. The non-WASM SSL path explicitly checks pending_plaintext() and returns those bytes before propagating either clean or unexpected EOF; this path needs the same ordering.

Useful? React with 👍 / 👎.

Applied. pump now feeds BIO EOF only when rustls has no pending plaintext, so a later read() still returns the leftover application data before ZeroReturn/EOF.
— commented by Claude

* address review: assignment scan and wasm TLS Skip comments and lambda defaults in the condition assignment scanner. Restrict the suite-header colon fallback to yield targets. Capture __del__ before calling it. Store browser socket timeouts as f64 and accept integers. Fail closed when wasm verify_mode is not CERT_NONE. Export getpeercert and feed MemoryBIO EOF. Assisted-by: Grok:grok-4.6 * address review: parenthesized yield suite header Walk through grouping parentheses before requiring yield after a suite-header colon. Assisted-by: Grok:grok-4.6

chatgpt-codex-connector Bot reviewed Sep 14, 2026

View reviewed changes

address review: parenthesized yield suite header

f7e2a34

Walk through grouping parentheses before requiring yield after a suite-header colon. Assisted-by: Grok:grok-4.6

chatgpt-codex-connector Bot reviewed Sep 14, 2026

View reviewed changes

youknowone merged commit ea02f84 into RustPython:main Sep 14, 2026
29 checks passed

youknowone deleted the host-env-win-ffi branch September 14, 2026 22:23

youknowone mentioned this pull request Sep 15, 2026

Honor asyncgen ag_closed and wasm BIO EOF #8718

Merged

Navigation Menu

Repository navigation

address review: assignment scan and wasm TLS - #8716

address review: assignment scan and wasm TLS#8716
youknowone merged 2 commits into
RustPython:mainfrom
youknowone:host-env-win-ffi

youknowone commented Sep 14, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

chatgpt-codex-connector Bot commented Sep 14, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Sep 14, 2026 •

edited

Loading

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Sep 14, 2026

Uh oh!

youknowone Sep 14, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Sep 14, 2026

Uh oh!

youknowone Sep 14, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

		} else if eof {
		conn.feed_tls(&[]).map_err(\|err\| map_tls(vm, err))?;

Conversation

youknowone commented Sep 14, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

chatgpt-codex-connector Bot commented Sep 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codex Review Summary

Uh oh!

coderabbitai Bot commented Sep 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Sep 14, 2026

Choose a reason for hiding this comment

Uh oh!

youknowone Sep 14, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Sep 14, 2026

Choose a reason for hiding this comment

Uh oh!

youknowone Sep 14, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

youknowone commented Sep 14, 2026 •

edited by coderabbitai Bot

Loading

chatgpt-codex-connector Bot commented Sep 14, 2026 •

edited

Loading

coderabbitai Bot commented Sep 14, 2026 •

edited

Loading