Skip to content

[mypyc] feat: unwrap NewType types to their base types for optimized code paths #19497

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 20 commits into
base: master
Choose a base branch
from

Conversation

BobTheBuidler
Copy link
Contributor

@BobTheBuidler BobTheBuidler commented Jul 24, 2025

This PR adds special case logic for unwrapping NewType types to their actual type. This logic is currently working. a NewType("name", str) now generates the same code as a str.

I wasn't entirely sure of the best way to test this, so I just tweaked the str tests to use a union of str and newtype str and validated that the IR still uses str and not object

Almost all of my tests are running fine, but I get a strange mypy error in the str.count tests saying that str objects do not have a .count method? Of course a str object has a .count method. Do you think this might related to the typeshed stuff again?

@BobTheBuidler
Copy link
Contributor Author

I'm just going to.... revert my change to the count() test so all of my tests pass and I get a green check on the PRs page

I don't think this count() issue implies an issue with the PR because all of the other string methods generate their C code properly.

Copy link
Collaborator

@sterliakov sterliakov left a comment

Choose a reason for hiding this comment

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

Nice patch, thanks!

@BobTheBuidler
Copy link
Contributor Author

I actually figured out last night how to fix the fixtures, so I'll do that instead for the sake of completeness

@BobTheBuidler
Copy link
Contributor Author

Is there a good way to define MyStr once at the top and reuse it in each of the test cases?

The current state is good enough but feels a bit cluttered for my liking. Unsure if an issuse.

@BobTheBuidler
Copy link
Contributor Author

@brianschubert updated accordingly

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