_BUILD_TUPLE is preventing us from optimizing out _POP_CALL_TWO_LOAD_CONST_INLINE_BORROW.
The bytecode is basically:

LOAD_CONST
LOAD_CONST
_BUILD_TUPLE
_POP_CALL_TWO_LOAD_CONST_INLINE_BORROW

To optimize this, we'd need some special handling for _BUILD_TUPLE in remove_unneeded_uops.

I'm not sure about this define, maybe it's fine to duplicate this logic?

I'll let you choose. We should probably either duplicate it, or just make it a proper function in optimizer_symbols.c.

We can't infer anything in this case, since the class could do anything in its __instancecheck__ or whatever, so it's no longer side-effect free. Need to bail on the whole optimization at this point.

So I don't think we need all_items_known, either. We can either:

• Break early on our first True, like you do below, and infer True.
• Loop over everything and infer False.
• Bail on an unknown thing and infer bool.

I guess we could still do something like this if we know sym_get_type(item) == &PyType_Type, so it's guaranteed side-effect-free, we just don't know the result of the test. But that seems like a rare case (knowing something is a type, but not which type it actually is).

We can't infer anything in this case, since the class could do anything in its instancecheck or whatever, so it's no longer side-effect free. Need to bail on the whole optimization at this point.

Just to check if I understood this point: even if we have something like isinstance(42, (unknown, int)) we can't infer True because if unknown defines its own __instancecheck__, it would change the semantics of the program if we infer True. This is because unknown.__instancecheck__ would no longer be called, right?

So basically what you said, once we see an item with an unknown type, we must stop.

Basically, yeah. It's a bit more subtle, though: we can infer the result to be True, but we can't remove the isinstance call itself.

So we can set a replace_op = false flag or something to keep the inferred value but avoid the REPLACE_OP call. That would at least allow us to remove the following branch in if isinstance(42, (has_metaclass, int)): ..., even if the isinstance call itself remains.

Basically, yeah. It's a bit more subtle, though: we can infer the result to be True, but we can't remove the isinstance call itself.

Awesome, that was pretty much my understanding as well :)

I'll fix the PR hopefully this weekend.

I'll let you choose. We should probably either duplicate it, or just make it a proper function in optimizer_symbols.c.