| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
|
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. |
Sorry, something went wrong.
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? |
Sorry, something went wrong.
|
I opened PR #136040 to fix the compact ints issue. |
Sorry, something went wrong.
Did you mean checking for _Py_GetConfig()->bytes_warning conditionally? I fear that this might be really slow :( |
Sorry, something went wrong.
You can fetch the value once at the start of optimization time in optimizer_analysis.c, and store it somewhere. |
Sorry, something went wrong.
|
Or, simpler, just explicitly disallow optimizing the problematic comparisons. |
Sorry, something went wrong.
|
If one of the values is of type bytes, require the other to be as well. |
Sorry, something went wrong.
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? |
Sorry, something went wrong.
|
What BINARY_OP will raise BytesWarning? |
Sorry, something went wrong.
Not BytesWarning. BINARY_OP (str, bytes) will throw. |
Sorry, something went wrong.
|
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). |
Sorry, something went wrong.
|
Same for 42 / 0, etc. In general, errors in the optimizer should just be suppressed. |
Sorry, something went wrong.
|
This is just different because it’s a warning, to clarify. |
Sorry, something went wrong.
Yeah the optimizer currently doesn't do that. Opened a PR to do so #136048 |
Sorry, something went wrong.
|
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 :( |
Sorry, something went wrong.
|
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 :)
Is there anything I need to do in this PR to address them? |
Sorry, something went wrong.
We plan to follow up shortly with COMPARE_OP here #130415 |
Sorry, something went wrong.
|
Ok then :) |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Uh oh!
There was an error while loading. Please reload this page.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.