Skip to content

[mypyc] add support for contextlib.contextmanager - #22267

Open
hauntsaninja wants to merge 8 commits into
python:masterfrom
hauntsaninja:context
Open

hauntsaninja wants to merge 8 commits into
python:masterfrom
hauntsaninja:context

Conversation

@hauntsaninja

@hauntsaninja hauntsaninja commented Oct 22, 2026 •

Copy link
Copy Markdown
Collaborator

Should make self check like 6-10% faster and torch 8-12% faster (compiled, clean cache, not parallel)

Fixes mypyc/mypyc#905

Co-authored by Codex

@hauntsaninja
hauntsaninja marked this pull request as ready for review October 22, 2026 04:10
@hauntsaninja
hauntsaninja requested a review from JukkaL October 22, 2026 04:42
@ilevkivskyi

Copy link
Copy Markdown
Member

I guess you can say that this fixes mypyc/mypyc#905, right? (Btw I didn't look in details, @JukkaL would be a better reviewer here.)

Comment thread mypyc/irbuild/statement.py Resolved
Comment thread mypyc/irbuild/statement.py Resolved
Comment on lines +1288 to +1295
wrapped_call = mypy.nodes.CallExpr(
mypy.nodes.MemberExpr(expr.callee, "__wrapped__"),
expr.args,
expr.arg_kinds,
expr.arg_names,
)
wrapped_call.line = line
gen = builder.accept(wrapped_call)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

i think this results in a generic Python attribute access + generic function call at runtime which we could optimize for compiled functions decorated with @contextmanager. for those we could call the generated C function directly.

assert isinstance(expr.callee, mypy.nodes.MemberExpr)
receiver = builder.accept(receiver_expr)
method = builder.py_get_attr(receiver, expr.callee.name, line)
wrapped = builder.py_get_attr(method, "__wrapped__", line)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

for non-final methods we should check and optimize only if __wrapped__ actually exists because it's legal to override a contextmanager method with a regular method in a subclass.

we might be able to optimize the call for final methods and for methods of compiled classes that don't allow interpreted subclasses by calling the C function directly.

Comment thread mypyc/test-data/run-functions.test Resolved
Comment thread mypyc/test-data/run-functions.test Resolved
Comment thread mypyc/test-data/run-functions.test Resolved
Comment thread mypyc/test-data/run-functions.test Resolved
@hauntsaninja

Copy link
Copy Markdown
Collaborator Author

Thanks for the review, will address!

I used prints instead of runtime asserts because the exception logic is actually kind of tricky and was a source of bugs for me, very easy to accidentally suppress an exception

I couldn't see an easy way to find the generated C function. Codex had a diff where it did something that created some new statics, it benchmarked only a little bit better, but you're right that it could be a little more robust. Let me investigate it.

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.

Specialize contextlib.contextmanager

3 participants