Skip to content

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?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think the current code goes to error on some cases where we run out of memory too, but no memoryerror is set.

Copy link
Member

Choose a reason for hiding this comment

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

I don't think so, all uses of goto error in these files seem like they have an error set.

@Fidget-Spinner
Copy link
Member Author

@tomasr8 may I trouble you with a review please?

Copy link
Member

@tomasr8 tomasr8 left a comment

Choose a reason for hiding this comment

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

Given Brandt's explanation, this looks good to me :) I just left one question/comment

@Fidget-Spinner Fidget-Spinner merged commit 46f823b into python:main Sep 15, 2025
58 checks passed
@Fidget-Spinner Fidget-Spinner deleted the clear-error-on-optimizer-failure branch September 15, 2025 16:24
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