Repository navigation
Behavior change for foo and 1 or 2: 3.12 newly converts foo to bool twice #124285
Description
Activity
I haven't had a chance to look closer at this, but I will say that flipping the truthiness of something everytime it's used in
__bool__is quite odd. How is this being used in practice?We noticed this when upgrading pytype to support 3.12. Pytype evaluates the bytecode and reported that this expression may now now be
Union[Foo, str, int]. See google/pytype#1777.After staring at the bytecode for a while we noticed this is really a runtime change and a specifically crafted
__bool__method could show that. But I don't know of any real example that does that.Apparently pyright also encodes that behavior somehow. But it does that independently of the Python version. pyright playground.
Edit: I was honestly wondering if this is worth filing an issue about. But ultimately I thought there is no harm in reporting.
Confirmed to be happening on current main as well
Bisected to 3468c76
cc @iritkatrielDocumentation in 3.12 says: "The expression x and y first evaluates x; if x is false, its value is returned; otherwise, y is evaluated and the resulting value is returned.
The expression x or y first evaluates x; if x is true, its value is returned; otherwise, y is evaluated and the resulting value is returned.
Note that neither and nor or restrict the value and type they return to False and True, but rather return the last evaluated argument."That's exactly how it works in 3.12+:
- Compute Foo() and "a string":
- Foo() evaluates to False => returns Foo() object
- Compute Foo() or "42":
- Foo() evaluated to False => returns Foo() object
# a.py class Foo: def __init__(self): self._a = True def __bool__(self): print(self) self._a = not self._a print(f"Foo.__bool__ -> {self._a}") return self._a print(Foo() and "a string" or 42)
$ python3.12 a.py <__main__.Foo object at 0x7f1b514d6120> Foo.__bool__ -> False <__main__.Foo object at 0x7f1b514d6120> Foo.__bool__ -> True <__main__.Foo object at 0x7f1b514d6120>It looks, that before the result of evaluation Foo() in (2) was "cached". That's ok, assuming that bool(obj) would have no side effects. Which is not our case:
>>> x = Foo() >>> bool(x) <a.Foo object at 0x7fb5d7bfbaa0> Foo.__bool__ -> False False >>> bool(x) <a.Foo object at 0x7fb5d7bfbaa0> Foo.__bool__ -> True True
@Eclips4, thanks for debugging. But I don't think it's a bug. Does documentation says somewhere that
__bool__()should have no side effects?Reacted by StefanIt's too late to change something as big as that and say that
__bool__should have no side effects. In fact, it's essential in Python to expect side effects somewhere.. This is not making__bool__an exclusive here. The fact that__bool__is called twice (which actually is a reason of the following) is less scary than the fact that the same code has different results on different versions, because the commit which introduced it didn't intend that.Reacted by Alex Waygood, Petr Viktorin, Ethan Furman and Jan Kaliszewskithe commit which introduced it didn't intend that.
Maybe (let's wait a reply from core dev;)), but IMHO it looks like it fixed a bug (which was mentioned by OP pre-3.12 behaviour).
If we return rules as in 3.11 - we will have to adjust docs accordingly, because old behaviour doesn't match docs (in 3.11 - too). And yes,
__bool__()- will be a special beast!Reacted by StefanIf we do decide to fix this, it's probably going to be too complicated to backport :(
It's a bug. Terms should only be evaluated once.
Reacted by sobolevn and Jan Kaliszewski3 remaining items
I merged a fix, but I don't know about back porting it.
@JelleZijlstra asked on the 3.14 PR if the idea of doing this in the AST optimiser was worth considering further.
Duplicating the
cnode in the generated AST really isn't desirable if we can get the same performance benefit later in the pipeline without introducing any duplication in the generated code.I mainly suggested the AST option in case it was hard to restore the optimisation at the opcode generation stage, which doesn't appear to be a relevant concern (since @iritkatriel already restored it).
Reacted by Jelle Zijlstra and Irit Katriel@iritkatriel I think we shouldn't backport this, both because the fix is invasive (new opcodes), and because the behavior change feels too big for a bugfix release.
Reacted by Alyssa CoghlanThis skip coincidentally came up in a recent forum thread.
The point was made that if compilers are allowed to make this assumption (and they are, since the reference interpreter did it for years, and only stopped due to a bug), then the
__bool__data model entry should mention that.Edit: given the bug fix is too invasive to reasonably backport, I think that means the remaining work here is choosing suitable docs wording for 3.12/3.13/3.14, deciding where it should live in the language reference, and then mentioning it in the docs for
__bool__(and maybe__len__?)Reacted by Stefan
Bug report
Bug description:
We noticed a behavior change between 3.11 and 3.12. The following code calls
Foo.__bool__once in 3.11 and twice in 3.12. Consequently for this contrived example, the expression evaluates to different results in 3.11 and 3.12.In Python 3.11:
In Python 3.12 (and 3.13.0b2):
Is this change intentional?
Note that I'm not necessarily asking to change this. We should arguably change the code to
"a string" if Foo() else 42, which evaluates the same in 3.11 and 3.12.CPython versions tested on:
3.12
Operating systems tested on:
Linux
Linked PRs