GeezerVison

One thing that slightly worries me about this is that comparing str/int and bytes objects raises UnicodeWarning if the -b option was passed on the command line. With this change, additional checks are required if we want to make COMPARE_OP able to constant-evaluate in the optimizer.

One thing that slightly worries me about this is that comparing str/int and bytes objects raises UnicodeWarning if the -b option was passed on the command line. With this change, additional checks are required if we want to make COMPARE_OP able to constant-evaluate in the optimizer.

Ok let's not do this then. We still need to fix the long stuff though so @sobolevn are you open to that or do you want me to handle it?

I opened PR #136040 to fix the compact ints issue.

With this change, additional checks are required if we want to make COMPARE_OP able to constant-evaluate in the optimizer.

Did you mean checking for _Py_GetConfig()->bytes_warning conditionally? I fear that this might be really slow :(

With this change, additional checks are required if we want to make COMPARE_OP able to constant-evaluate in the optimizer.

Did you mean checking for _Py_GetConfig()->bytes_warning conditionally? I fear that this might be really slow :(

You can fetch the value once at the start of optimization time in optimizer_analysis.c, and store it somewhere.

Or, simpler, just explicitly disallow optimizing the problematic comparisons.

If one of the values is of type bytes, require the other to be as well.

Or, simpler, just explicitly disallow optimizing the problematic comparisons.

Not just comparisons. BINARY_OP as well. This is a little annoying and more involved than I expected, so I'd say let's just ignore it for now?

What BINARY_OP will raise BytesWarning?

What BINARY_OP will raise BytesWarning?

Not BytesWarning. BINARY_OP (str, bytes) will throw.

That’s the case for many combinations of safe types. We should suppress the error and hit bottom in that case (in the error label).

Same for 42 / 0, etc. In general, errors in the optimizer should just be suppressed.

This is just different because it’s a warning, to clarify.

That’s the case for many combinations of safe types. We should suppress the error and hit bottom in that case (in the error label).

Yeah the optimizer currently doesn't do that. Opened a PR to do so #136048

I opened https://discuss.python.org/t/consider-deprecating-and-eventually-removing-b-cli-flag/96903 about possibly removing this warning in the future versions. It does not seem very useful :(

I fixed the merge conflict. Sorry, I am rather new to the JIT internals, and I don't quite understand the COMPARE_OP part. _Py_uop_sym_is_safe_const is never called for COMPARE_OP. So, I don't know how to interpret these comments :)

With this change, additional checks are required if we want to make COMPARE_OP able to constant-evaluate in the optimizer.

Or, simpler, just explicitly disallow optimizing the problematic comparisons.

Is there anything I need to do in this PR to address them?

I fixed the merge conflict. Sorry, I am rather new to the JIT internals, and I don't quite understand the COMPARE_OP part. _Py_uop_sym_is_safe_const is never called for COMPARE_OP. So, I don't know how to interpret these comments :)

We plan to follow up shortly with COMPARE_OP here #130415

Ok then :)
Let's close it for now.