Skip to content

gh-132732: Clear errors in JIT optimizer on error #136048

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 3 commits into
base: main
Choose a base branch
from

Conversation

Fidget-Spinner
Copy link
Member

@Fidget-Spinner Fidget-Spinner commented Jun 27, 2025

@tomasr8
Copy link
Member

tomasr8 commented Jul 1, 2025

Asking just to educate myself: what kind of errors are we expecting inside the JIT and is it safe to always clear them?

@brandtbucher
Copy link
Member

brandtbucher commented Jul 1, 2025

In general, we want to shift as much computation as possible to optimization-time instead of runtime. Some classes of errors, like MemoryError, are unavoidable. In these cases, we can just ignore them and trust that they'll either resolve themselves or surface in normal user code soon (it's a bit confusing if we raise an error like this in seemingly "random" places where JIT compilation may kick in).

The other class of errors are ones that are easier to ask forgiveness than permission for. For example, constant-evaluating stuff like a / b, a % b, a >> b, a << b, a[b], and so on, where a and b are known "safe" constants in the JIT. It's way easier to just try the operation and assume it will succeed, and just walk back any errors when it doesn't (in this latter case, the error will just happen at runtime when the operation is evaluated "for real").

Comment on lines +562 to +564
if (PyErr_Occurred()) {
PyErr_Clear();
}
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be:

Suggested change
if (PyErr_Occurred()) {
PyErr_Clear();
}
assert(PyErr_Occurred());
PyErr_Clear();

...or are there cases where an error hasn't occurred, but we end up here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants