GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — No alloc in lookup_alias/lookup_named_sequence by joshuamegnauth54 · Pull Request #9819 · RustPython/RustPython

Conversation

@joshuamegnauth54

@joshuamegnauth54 joshuamegnauth54 commented Nov 12, 2026 •
edited by coderabbitai Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

The original code allocated for case insensitive comparison. Instead, I iterate through both strings' bytes and convert the bytes to uppercase for the comparison. This has the added benefit of only iterating once in total. The old code iterated at least three times (once per each string pair and then a third time to compare).

  • Closes #xxxx

One of checkbox below must be checked.

  • I did not use AI to write the code of this patch.
  • This PR follows our AI policy

Summary by CodeRabbit

  • Refactor
    • Unicode name and alias comparisons now avoid creating temporary uppercase strings during lookup. Case-insensitive matching behavior remains unchanged.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@joshuamegnauth54
No alloc in lookup_alias/lookup_named_sequence
0d005a6 
The original code allocated for case insensitive comparison. Instead, I
iterate through both strings' bytes and convert the bytes to uppercase
for the comparison. This has the added benefit of only iterating once in
total. The old code iterated at least three times (once per each string
pair and then a third time to compare).
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 12, 2026

Copy link
Copy Markdown
Codex Review Summary: ❌ Failed

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

Review Status Commit Review trigger
📝 Code Review ❌ Failed 2026-11-12T17:59:55.023697Z
We couldn’t complete this review. Comment @codex review to try again.		 0d005a6 PR opened
ℹ️ 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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 12, 2026

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@coderabbitai

coderabbitai Bot commented Nov 12, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

Alias and named-sequence binary searches now compare names with ASCII-uppercased byte iterators. They no longer build uppercased String keys.

Changes: Unicode name lookups

Layer / File(s) Summary
Compare names without allocating folded strings
crates/unicode/src/data.rs		 cmp_caseless compares ASCII-uppercased bytes. Alias and named-sequence binary searches use this comparison. The searches retain case-insensitive ASCII ordering.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Refactor

Suggested reviewers: youknowone

Merge Risk: Minimal · up to 0d005

This change removes allocations from alias and named-sequence lookups without altering the ordering contract. No actionable merge risk remains.

Pre-merge checks: Passed 4

Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title "No alloc in lookup_alias/lookup_named_sequence" names the two functions whose case-insensitive comparison changed. The raw summary says the binary searches now compare ASCII-uppercased byte…
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.
Full details: Title check

Explanation

The title "No alloc in lookup_alias/lookup_named_sequence" names the two functions whose case-insensitive comparison changed. The raw summary says the binary searches now compare ASCII-uppercased byte iterators directly instead of building an uppercased String key, which removes the allocation the title describes. The title states the main goal and names the affected functions, so it fully matches the changeset.

Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

  • X
  • Mastodon
  • Reddit
  • LinkedIn
Recent review info

Run configuration

  • Source mode: Auto
  • Repository: RustPython/RustPython
  • Branch: main
  • File: .coderabbit.yml
  • Commit: dba7a05
  • Layers: Repository YAML
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 6ddd6000-fd40-4f00-85be-3243295a9dcc
Commits

Reviewing files that changed from the base of the PR and between dba7a05 and 0d005a6.

Files selected for processing: 1

  • crates/unicode/src/data.rs

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

1 participant

@joshuamegnauth54
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.