Repository navigation
Behavior change for foo and 1 or 2: 3.12 newly converts foo to bool twice #124285
Description
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.
class Foo:
def __init__(self):
self._a = True
def __bool__(self):
self._a = not self._a
print(f"Foo.__bool__ -> {self._a}")
return self._a
Foo() and "a string" or 42In Python 3.11:
>>> Foo() and "a string" or 42 Foo.__bool__ -> False 42
In Python 3.12 (and 3.13.0b2):
>>> Foo() and "a string" or 42 Foo.__bool__ -> False Foo.__bool__ -> True <__main__.Foo object at 0x7f0ae554c1a0>
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
Activity
ZeroIntensity commented on Sep 20, 2024
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?
frigus02 commented on Sep 20, 2024
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.
Eclips4 commented on Sep 20, 2024
Yeah, this code is quite odd but this is definitely regression, thanks for the report @frigus02. I'm bisecting the bad commit right know
tomasr8 commented on Sep 20, 2024
Confirmed to be happening on current main as well
Eclips4 commented on Sep 20, 2024
Bisected to 3468c76
cc @iritkatriel
skirpichev commented on Sep 20, 2024
Documentation 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+:
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:
@Eclips4, thanks for debugging. But I don't think it's a bug. Does documentation says somewhere that __bool__() should have no side effects?
Eclips4 commented on Sep 20, 2024
It'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.
skirpichev commented on Sep 20, 2024
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!
ZeroIntensity commented on Sep 20, 2024
If we do decide to fix this, it's probably going to be too complicated to backport :(
ethanfurman commented on Sep 20, 2024
It's a bug. Terms should only be evaluated once.
3 remaining items
pythongh-124285: Fix bug where bool() is called multiple times for th…
gh-124285: Fix bug where bool() is called multiple times for the same…
iritkatriel commented on Sep 25, 2024
I merged a fix, but I don't know about back porting it.
ncoghlan commented on Sep 26, 2024
@JelleZijlstra asked on the 3.14 PR if the idea of doing this in the AST optimiser was worth considering further.
Duplicating the c node 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).
JelleZijlstra commented on Sep 26, 2024
@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.
ncoghlan commented on Sep 27, 2024
This 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__?)
pythongh-124285: document assumptions on __bool__/__len__ behaviour
skirpichev commented on Sep 28, 2024
Here is an attempt to document things: #124723
PS: @Eclips4, probably we should clear 3.12-3.14 labels, as the fix is not going to be backported?