Skip to content

gh-132732: Automatically constant evaluate pure operations #132733

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

Merged
merged 58 commits into from
Jun 27, 2025

Conversation

Fidget-Spinner
Copy link
Member

@Fidget-Spinner Fidget-Spinner commented Apr 19, 2025

@python-cla-bot
Copy link

python-cla-bot bot commented Apr 19, 2025

All commit authors signed the Contributor License Agreement.

CLA signed

Copy link
Member

@brandtbucher brandtbucher left a comment

Choose a reason for hiding this comment

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

This is really neat!

Other than two opcodes I found that shouldn't be marked pure, I just have one thought:

Rather than rewriting the bodies like this to use the symbols-manipulating functions (which seems error-prone), would we be able to just use stackrefs to do this?

For example, _BINARY_OP_ADD_INT is defined like this:

PyObject *left_o = PyStackRef_AsPyObjectBorrow(left);
PyObject *right_o = PyStackRef_AsPyObjectBorrow(right);
// ...
res = PyStackRef_FromPyObjectSteal(res_o);

Rather than rewriting uses of these functions, could it be easier to just do something like this, since we're guranteed not to escape?

if (sym_is_const(ctx, stack_pointer[-2]) && sym_is_const(ctx, stack_pointer[-1])) {
    // Generated code to turn constant symbols into stackrefs:
    _PyStackRef left = PyStackRef_FromPyObjectBorrow(sym_get_const(ctx, stack_pointer[-2]));
    _PyStackRef right = PyStackRef_FromPyObjectBorrow(sym_get_const(ctx, stack_pointer[-1]));
    _PyStackRef res;
    // Now the actual body, same as it appears in executor_cases.c.h:
    PyObject *left_o = PyStackRef_AsPyObjectBorrow(left);
    PyObject *right_o = PyStackRef_AsPyObjectBorrow(right);
    // ...
    res = PyStackRef_FromPyObjectSteal(res_o);
    // Generated code to turn stackrefs into constant symbols:
    stack_pointer[-1] = sym_new_const(ctx, PyStackRef_AsPyObjectSteal(res));
}

I'm not too familiar with the design of the cases generator though, so maybe this is way harder or something. Either way, I'm excited to see this get in!

@Fidget-Spinner
Copy link
Member Author

Rather than rewriting uses of these functions, could it be easier to just do something like this, since we're guranteed not to escape?

Seems feasible. I could try to rewrite all occurences of the variable with a stackref-producing const one. Let me try that.

@Fidget-Spinner
Copy link
Member Author

I've verified no refleak on test_capi.test_opt locally apart from #132731 which is pre-existing.

@markshannon
Copy link
Member

There's a lot going on in this PR, probably too much for one PR.

Could we start with a PR to fix up the pure annotations so that they are on the correct instructions and maybe add the pure_guard annotation that Brandt suggested?

@markshannon
Copy link
Member

Could we have the default code generator generate a function for the body of the pure instruction and then call that from the three interpreters?

@brandtbucher
Copy link
Member

Could we have the default code generator generate a function for the body of the pure instruction and then call that from the three interpreters?

Hm, I think I’d prefer not to. Sounds like it could hurt performance, especially for the JIT (where things can’t inline).

@brandtbucher
Copy link
Member

I think a good progression would be:

  • Implement the pure attribute, and the optimizer changes. Remove the pure attributes where they don’t belong (so nothing breaks) and leave the existing ones as proof that the implementation works. (This PR)
  • Audit the existing non-pure bytecodes and add pure where it makes sense. (Follow-up PR)
  • Implement the pure_guard attribute, and annotate any bytecodes that can use it. (Follow-up PR)

@Fidget-Spinner
Copy link
Member Author

Could we have the default code generator generate a function for the body of the pure instruction and then call that from the three interpreters?

Hm, I think I’d prefer not to. Sounds like it could hurt performance, especially for the JIT (where things can’t inline).

I thought about this and I think we can inline if we autogenerate a header file and include that directly. But then we're at the mercy of the compiler in both the normal interpreter and the JIT deciding to inline or not to inline the body again. Which I truly do not want.

@Fidget-Spinner
Copy link
Member Author

@brandtbucher @markshannon what can I do to get this PR moving?

@tomasr8 if youd like to review, here's a summary of the PR:

  1. If a bytecode operation is pure (no side effects) we can mark it as pure in bytecodes.c.
  2. In the optimizer, we automatically generate the body that does evaluation of the symbolic constants by copy pasting the bytecodes.c definition into the optimizer's C code. Of course we check that the inputs are constants first.
  3. All changes to the cases generator is for the second point.

@tomasr8
Copy link
Member

tomasr8 commented May 8, 2025

Thanks for the ping! I actually wanted to try/review this PR, I was just very busy this week with work :/ I'll have a look this weekend :)

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.

Only had time to skim the PR, I'll do a more thorough review this weekend :)

Co-Authored-By: Tomas R. <tomas.roun8@gmail.com>
@Fidget-Spinner
Copy link
Member Author

Does this work for BINARY_OP? I note that you haven't included it.

It should, let me try

@markshannon
Copy link
Member

That sadly wouldn't work for the next thing we plan to add this to: _COMPARE_OP_X ...

REPLACE_OPCODE_IF_EVALUATES_PURE(left, right, compare_ops[oparg>>5]) should work. Just make a little table of comparison functions.

Doesn't that defeat the purpose of this DSL addition, because we'd have to modify bytecodes.c just to get optimizer_bytecodes.c to optimize? The whole point is to not require the user to modify bytecodes.c or copy anything from there.

No need to modify bytecodes.c just put the helpers in optimizer_analysis.c. They can be static.

I know that doing this all automatically seems purer and more elegant, and it probably is, but the code generator is a bit of a pain point in terms of maintenance. If we can keep it simple with a bit of extra work elsewhere it is usually worth it.

One other thing. Ultimately we will want to move this functionality out of the optimizer_bytecodes.c into the mythical later partial evaluation pass. So keeping things simple should help with that.

@markshannon
Copy link
Member

In case it wasn't clear. The helper functions would be:
PyObject *compare_op_0(PyObject *a, PyObject *b) { return PyObject_RichCompare(a, b, 0) }
and so on.

@python python deleted a comment from Rk13-gold Jun 20, 2025
@Fidget-Spinner
Copy link
Member Author

In case it wasn't clear. The helper functions would be: PyObject *compare_op_0(PyObject *a, PyObject *b) { return PyObject_RichCompare(a, b, 0) } and so on.

Is this the same as this?

If you're worried about the bulk of the generated code, I can open an issue for someone to make a follow up PR to generate function templates from the bytecodes.c file, similar to what we already do for the tail calling interpreter. This would allow us to just call the function. That should go into a follow-up commit though, because it requires changes that affect more than just the optimizer (it will also touch executor_cases and such).

Copy link
Member

@markshannon markshannon left a comment

Choose a reason for hiding this comment

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

Once skip_to is removed from generators_common, this becomes nicely self contained.

With it properly encapsulated, it should be easy to modify in the future should we decide to use a slightly different approach.

I've a few more suggestions for tidying things up, but nothing fundamental.

@@ -829,7 +829,7 @@ dummy_func(
DEOPT_IF(!res);
}

pure op(_BINARY_OP_EXTEND, (descr/4, left, right -- res)) {
pure op(_BINARY_OP_EXTEND, (descr/4, left, right -- res)) {
Copy link
Member

Choose a reason for hiding this comment

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

This isn't "pure". It can do anything

@@ -86,6 +86,16 @@ def emit_to(out: CWriter, tkn_iter: TokenIterator, end: str) -> Token:
out.emit(tkn)
raise analysis_error(f"Expecting {end}. Reached end of file", tkn)

def skip_to(tkn_iter: TokenIterator, end: str) -> Token:
Copy link
Member

Choose a reason for hiding this comment

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

This is only used in the optimizer emitter (and I don't think it is needed there anyway)

)
from generators_common import (
DEFAULT_INPUT,
ROOT,
write_header,
Emitter,
TokenIterator,
skip_to,
Copy link
Member

Choose a reason for hiding this comment

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

See earlier comment.

) -> bool:
skip_to(tkn_iter, "SEMI")

emitter = OptimizerConstantEmitter(self.out, {}, self.original_uop, copy.deepcopy(self.stack))
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
emitter = OptimizerConstantEmitter(self.out, {}, self.original_uop, copy.deepcopy(self.stack))
emitter = OptimizerConstantEmitter(self.out, {}, self.original_uop, self.stack.copy())

@@ -4,6 +4,7 @@
"""

import argparse
import copy
Copy link
Member

Choose a reason for hiding this comment

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

Stacks already support copying efficiently and correctly.

storage: Storage,
inst: Instruction | None,
) -> bool:
skip_to(tkn_iter, "SEMI")
Copy link
Member

Choose a reason for hiding this comment

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

If you're just going to throw away the arguments, why have them?
Either check the names match or, better still, don't require the inputs at all.

return not unconditional


def replace_opcode_if_evaluates_pure_identifiers(uop: Uop) -> list[str]:
Copy link
Member

Choose a reason for hiding this comment

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

You already have the input names from the uop, [item.name for item in uop.stack.inputs]

Copy link
Member Author

Choose a reason for hiding this comment

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

No this is the input names from the REPLACE_OPCODE_IF_EVALUATES_PURE macro. However, now that you mentioned it, I should get the names from the replacement function, not from this. So thanks for pointing it out.

@@ -175,13 +369,14 @@ def write_uop(
cast = f"uint{cache.size*16}_t"
out.emit(f"{type}{cache.name} = ({cast})this_instr->operand0;\n")
if override:
emitter = OptimizerEmitter(out, {})
emitter = OptimizerEmitter(out, {}, uop, copy.deepcopy(stack))
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
emitter = OptimizerEmitter(out, {}, uop, copy.deepcopy(stack))
emitter = OptimizerEmitter(out, {}, uop, stack.copy())

@@ -75,6 +80,10 @@ def type_name(var: StackItem) -> str:
return "JitOptRef *"
return "JitOptRef "

def stackref_type_name(var: StackItem) -> str:
if var.is_array():
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if var.is_array():
assert not var.is_array(), "Unsafe to convert a symbol to an array-like StackRef."

Check this in the caller if that makes sense, and raise an analysis error there so we aren't relying on assert to handle input errors.

@bedevere-app
Copy link

bedevere-app bot commented Jun 27, 2025

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

@Fidget-Spinner
Copy link
Member Author

Thanks Mark for the review. The code is a lot cleaner now and self-contained.

I have made the requested changes; please review again

@bedevere-app
Copy link

bedevere-app bot commented Jun 27, 2025

Thanks for making the requested changes!

@markshannon, @tomasr8: please review the changes made to this pull request.

@bedevere-app bedevere-app bot requested review from markshannon and tomasr8 June 27, 2025 09:51
Copy link
Member

@markshannon markshannon left a comment

Choose a reason for hiding this comment

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

That's much better. Thanks.

@Fidget-Spinner Fidget-Spinner merged commit 695ab61 into python:main Jun 27, 2025
69 checks passed
@Fidget-Spinner Fidget-Spinner deleted the pure branch June 27, 2025 11:37
@bedevere-bot
Copy link

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot iOS ARM64 Simulator 3.x (tier-3) has failed when building commit 695ab61.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/1380/builds/4083) and take a look at the build logs.
  4. Check if the failure is related to this commit (695ab61) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#/builders/1380/builds/4083

Failed tests:

  • test_socketserver

Failed subtests:

  • test_ThreadingUnixDatagramServer - test.test_socketserver.SocketServerTest.test_ThreadingUnixDatagramServer
  • test_UnixDatagramServer - test.test_socketserver.SocketServerTest.test_UnixDatagramServer

Summary of the results of the build (if available):

==

Click to see traceback logs
Traceback (most recent call last):
  File "/Users/buildbot/Library/Developer/XCTestDevices/3299090E-A190-41F9-9EFB-E7EB12DA3473/data/Containers/Bundle/Application/EDDB3F71-0E9C-40F0-97D8-EB8A643BB963/iOSTestbed.app/python/lib/python3.15/test/test_socketserver.py", line 222, in test_UnixDatagramServer
    self.run_server(socketserver.UnixDatagramServer,
    ~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                    socketserver.DatagramRequestHandler,
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                    self.dgram_examine)
                    ^^^^^^^^^^^^^^^^^^^
  File "/Users/buildbot/Library/Developer/XCTestDevices/3299090E-A190-41F9-9EFB-E7EB12DA3473/data/Containers/Bundle/Application/EDDB3F71-0E9C-40F0-97D8-EB8A643BB963/iOSTestbed.app/python/lib/python3.15/test/support/threading_helper.py", line 66, in decorator
    return func(*args)
  File "/Users/buildbot/Library/Developer/XCTestDevices/3299090E-A190-41F9-9EFB-E7EB12DA3473/data/Containers/Bundle/Application/EDDB3F71-0E9C-40F0-97D8-EB8A643BB963/iOSTestbed.app/python/lib/python3.15/test/test_socketserver.py", line 133, in run_server
    testfunc(svrcls.address_family, addr)
    ~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/buildbot/Library/Developer/XCTestDevices/3299090E-A190-41F9-9EFB-E7EB12DA3473/data/Containers/Bundle/Application/EDDB3F71-0E9C-40F0-97D8-EB8A643BB963/iOSTestbed.app/python/lib/python3.15/test/test_socketserver.py", line 160, in dgram_examine
    buf = data = receive(s, 100)
                 ~~~~~~~^^^^^^^^
  File "/Users/buildbot/Library/Developer/XCTestDevices/3299090E-A190-41F9-9EFB-E7EB12DA3473/data/Containers/Bundle/Application/EDDB3F71-0E9C-40F0-97D8-EB8A643BB963/iOSTestbed.app/python/lib/python3.15/test/test_socketserver.py", line 43, in receive
    raise RuntimeError("timed out on %r" % (sock,))
RuntimeError: timed out on <socket.socket fd=23, family=1, type=2, proto=0, laddr=./test_python_x18vehwp.sock>


Traceback (most recent call last):
  File "/Users/buildbot/Library/Developer/XCTestDevices/3299090E-A190-41F9-9EFB-E7EB12DA3473/data/Containers/Bundle/Application/EDDB3F71-0E9C-40F0-97D8-EB8A643BB963/iOSTestbed.app/python/lib/python3.15/threading.py", line 1074, in _bootstrap_inner
    self._context.run(self.run)
    ~~~~~~~~~~~~~~~~~^^^^^^^^^^
  File "/Users/buildbot/Library/Developer/XCTestDevices/3299090E-A190-41F9-9EFB-E7EB12DA3473/data/Containers/Bundle/Application/EDDB3F71-0E9C-40F0-97D8-EB8A643BB963/iOSTestbed.app/python/lib/python3.15/threading.py", line 1016, in run
    self._target(*self._args, **self._kwargs)
    ~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/buildbot/Library/Developer/XCTestDevices/3299090E-A190-41F9-9EFB-E7EB12DA3473/data/Containers/Bundle/Application/EDDB3F71-0E9C-40F0-97D8-EB8A643BB963/iOSTestbed.app/python/lib/python3.15/socketserver.py", line 240, in serve_forever
    self._handle_request_noblock()
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^
  File "/Users/buildbot/Library/Developer/XCTestDevices/3299090E-A190-41F9-9EFB-E7EB12DA3473/data/Containers/Bundle/Application/EDDB3F71-0E9C-40F0-97D8-EB8A643BB963/iOSTestbed.app/python/lib/python3.15/socketserver.py", line 320, in _handle_request_noblock
    self.handle_error(request, client_address)
    ~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/buildbot/Library/Developer/XCTestDevices/3299090E-A190-41F9-9EFB-E7EB12DA3473/data/Containers/Bundle/Application/EDDB3F71-0E9C-40F0-97D8-EB8A643BB963/iOSTestbed.app/python/lib/python3.15/socketserver.py", line 318, in _handle_request_noblock
    self.process_request(request, client_address)
    ~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/buildbot/Library/Developer/XCTestDevices/3299090E-A190-41F9-9EFB-E7EB12DA3473/data/Containers/Bundle/Application/EDDB3F71-0E9C-40F0-97D8-EB8A643BB963/iOSTestbed.app/python/lib/python3.15/socketserver.py", line 349, in process_request
    self.finish_request(request, client_address)
    ~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/buildbot/Library/Developer/XCTestDevices/3299090E-A190-41F9-9EFB-E7EB12DA3473/data/Containers/Bundle/Application/EDDB3F71-0E9C-40F0-97D8-EB8A643BB963/iOSTestbed.app/python/lib/python3.15/socketserver.py", line 362, in finish_request
    self.RequestHandlerClass(request, client_address, self)
    ~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/buildbot/Library/Developer/XCTestDevices/3299090E-A190-41F9-9EFB-E7EB12DA3473/data/Containers/Bundle/Application/EDDB3F71-0E9C-40F0-97D8-EB8A643BB963/iOSTestbed.app/python/lib/python3.15/socketserver.py", line 768, in __init__
    self.finish()
    ~~~~~~~~~~~^^
  File "/Users/buildbot/Library/Developer/XCTestDevices/3299090E-A190-41F9-9EFB-E7EB12DA3473/data/Containers/Bundle/Application/EDDB3F71-0E9C-40F0-97D8-EB8A643BB963/iOSTestbed.app/python/lib/python3.15/socketserver.py", line 863, in finish
    self.socket.sendto(self.wfile.getvalue(), self.client_address)
    ~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
ConnectionResetError: [Errno 54] Connection reset by peer
ERROR


Traceback (most recent call last):
  File "/Users/buildbot/Library/Developer/XCTestDevices/3299090E-A190-41F9-9EFB-E7EB12DA3473/data/Containers/Bundle/Application/EDDB3F71-0E9C-40F0-97D8-EB8A643BB963/iOSTestbed.app/python/lib/python3.15/test/test_socketserver.py", line 228, in test_ThreadingUnixDatagramServer
    self.run_server(socketserver.ThreadingUnixDatagramServer,
    ~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                    socketserver.DatagramRequestHandler,
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
                    self.dgram_examine)
                    ^^^^^^^^^^^^^^^^^^^
  File "/Users/buildbot/Library/Developer/XCTestDevices/3299090E-A190-41F9-9EFB-E7EB12DA3473/data/Containers/Bundle/Application/EDDB3F71-0E9C-40F0-97D8-EB8A643BB963/iOSTestbed.app/python/lib/python3.15/test/support/threading_helper.py", line 66, in decorator
    return func(*args)
  File "/Users/buildbot/Library/Developer/XCTestDevices/3299090E-A190-41F9-9EFB-E7EB12DA3473/data/Containers/Bundle/Application/EDDB3F71-0E9C-40F0-97D8-EB8A643BB963/iOSTestbed.app/python/lib/python3.15/test/test_socketserver.py", line 133, in run_server
    testfunc(svrcls.address_family, addr)
    ~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/buildbot/Library/Developer/XCTestDevices/3299090E-A190-41F9-9EFB-E7EB12DA3473/data/Containers/Bundle/Application/EDDB3F71-0E9C-40F0-97D8-EB8A643BB963/iOSTestbed.app/python/lib/python3.15/test/test_socketserver.py", line 160, in dgram_examine
    buf = data = receive(s, 100)
                 ~~~~~~~^^^^^^^^
  File "/Users/buildbot/Library/Developer/XCTestDevices/3299090E-A190-41F9-9EFB-E7EB12DA3473/data/Containers/Bundle/Application/EDDB3F71-0E9C-40F0-97D8-EB8A643BB963/iOSTestbed.app/python/lib/python3.15/test/test_socketserver.py", line 43, in receive
    raise RuntimeError("timed out on %r" % (sock,))
RuntimeError: timed out on <socket.socket fd=23, family=1, type=2, proto=0, laddr=./test_python_a8m4t39h.sock>


Traceback (most recent call last):
  File "/Users/buildbot/Library/Developer/XCTestDevices/3299090E-A190-41F9-9EFB-E7EB12DA3473/data/Containers/Bundle/Application/EDDB3F71-0E9C-40F0-97D8-EB8A643BB963/iOSTestbed.app/python/lib/python3.15/threading.py", line 1074, in _bootstrap_inner
    self._context.run(self.run)
    ~~~~~~~~~~~~~~~~~^^^^^^^^^^
  File "/Users/buildbot/Library/Developer/XCTestDevices/3299090E-A190-41F9-9EFB-E7EB12DA3473/data/Containers/Bundle/Application/EDDB3F71-0E9C-40F0-97D8-EB8A643BB963/iOSTestbed.app/python/lib/python3.15/threading.py", line 1016, in run
    self._target(*self._args, **self._kwargs)
    ~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/buildbot/Library/Developer/XCTestDevices/3299090E-A190-41F9-9EFB-E7EB12DA3473/data/Containers/Bundle/Application/EDDB3F71-0E9C-40F0-97D8-EB8A643BB963/iOSTestbed.app/python/lib/python3.15/socketserver.py", line 699, in process_request_thread
    self.handle_error(request, client_address)
    ~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/buildbot/Library/Developer/XCTestDevices/3299090E-A190-41F9-9EFB-E7EB12DA3473/data/Containers/Bundle/Application/EDDB3F71-0E9C-40F0-97D8-EB8A643BB963/iOSTestbed.app/python/lib/python3.15/socketserver.py", line 697, in process_request_thread
    self.finish_request(request, client_address)
    ~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/buildbot/Library/Developer/XCTestDevices/3299090E-A190-41F9-9EFB-E7EB12DA3473/data/Containers/Bundle/Application/EDDB3F71-0E9C-40F0-97D8-EB8A643BB963/iOSTestbed.app/python/lib/python3.15/socketserver.py", line 362, in finish_request
    self.RequestHandlerClass(request, client_address, self)
    ~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/buildbot/Library/Developer/XCTestDevices/3299090E-A190-41F9-9EFB-E7EB12DA3473/data/Containers/Bundle/Application/EDDB3F71-0E9C-40F0-97D8-EB8A643BB963/iOSTestbed.app/python/lib/python3.15/socketserver.py", line 768, in __init__
    self.finish()
    ~~~~~~~~~~~^^
  File "/Users/buildbot/Library/Developer/XCTestDevices/3299090E-A190-41F9-9EFB-E7EB12DA3473/data/Containers/Bundle/Application/EDDB3F71-0E9C-40F0-97D8-EB8A643BB963/iOSTestbed.app/python/lib/python3.15/socketserver.py", line 863, in finish
    self.socket.sendto(self.wfile.getvalue(), self.client_address)
    ~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
ConnectionResetError: [Errno 54] Connection reset by peer
Warning -- threading_cleanup() failed to clean up threads in 1.0 seconds
Warning --   before: thread count=0, dangling=1
Warning --   after: thread count=1, dangling=4
Warning -- Dangling thread: <_MainThread(MainThread, started 4302897664)>
Warning -- Dangling thread: <Thread(Thread-2933 (process_request_thread), stopped 6218264576)>
Warning -- Dangling thread: <Thread(Thread-2932 (process_request_thread), stopped 6201438208)>
Warning -- Dangling thread: <Thread(<class 'socketserver.ThreadingUnixDatagramServer'> serving, started daemon 6184611840)>
ERROR

return (typ == &PyLong_Type) ||
(typ == &PyUnicode_Type) ||
(typ == &PyFloat_Type) ||
(typ == &PyTuple_Type) ||
Copy link
Member

Choose a reason for hiding this comment

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

@Fidget-Spinner, tuple isn't safe for comparisons either. The contained items may escape when compared.

We could check the items recursively for safety using sym_tuple_getitem. Or just remove this.

I'm leaning slightly towards the recursive check, but either could make sense.

Copy link
Member

Choose a reason for hiding this comment

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

We should also add None here, as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok let's remove 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.

7 participants