FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-124285: document assumptions on __bool__/__len__ behaviour by skirpichev · Pull Request #124723 · python/cpython · GitHub

/ cpython Public

gh-124285: document assumptions on __bool__/__len__ behaviour #124723

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.

Sign up for GitHub

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

Closed
skirpichev wants to merge 1 commit into from
Conversation 20 Commits 1 Checks 0 Files changed
Closed

gh-124285: document assumptions on __bool__/__len__ behaviour #124723

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension .rst  (1) All 1 file type selected
Viewed files
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Unified
Split
Hide whitespace
Diff view
Unified
Split
Hide whitespace
8 changes: 7 additions & 1 deletion Doc/reference/datamodel.rst
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -2037,12 +2037,15 @@ Basic customization
.. index:: single: __len__() (mapping object method)

Called to implement truth value testing and the built-in operation
``bool()``; should return ``False`` or ``True``. When this method is not
:func:`bool`; should return ``False`` or ``True``. When this method is not
defined, :meth:`~object.__len__` is called, if it is defined, and the object is
considered true if its result is nonzero. If a class defines neither
:meth:`!__len__` nor :meth:`!__bool__`, all its instances are considered
true.

Two successive calls to :meth:`!__bool__` on the same object must
return same value.

iritkatriel Sep 28, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Not if the object is mutated.

skirpichev Sep 28, 2024

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

But that means - some other method was called on the object in between. Thus, calls to __bool__ weren't actually successive.

AlexWaygood Sep 28, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

The object could have been mutated by another thread between the two calls...

skirpichev Sep 29, 2024

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

by another thread

... that calls some other object's method

between the two calls...

:)

skirpichev Sep 29, 2024 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Hmm, how about this: "The __bool__ method can't mutate any objects."? (Ditto for __len__.)

This probably is a more strong requirement than actually need in current optimizations, but I doubt it blocks something useful.

Comment on lines +2046 to +2047

ncoghlan Oct 6, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality
Suggested change
Two successive calls to :meth:`!__bool__` on the same object must
return same value.
Note: to help optimize logical expressions, implementations are permitted to assume
that calls to :meth:`!__bool__` will not mutate that object, nor any other object.
While this expected invariant is not explicitly enforced, failing to abide
by it will result in implementation dependent runtime behaviour. This
implementation dependent behaviour may also be encountered when mutable
objects are shared across threads without appropriate synchronization.

willingc Oct 6, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I'm unsure if this needs to be explicitly documented as the behavior is dependent on the user's choice of implementation.

skirpichev Oct 7, 2024

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

is dependent on the user's choice of implementation.

Rather on bugs in the user code, if someone will implement __bool__(), doing crazy things (see issue). Added docs say that implementation may assume certain behaviour from the user code, just as for the __hash__() method (same hash value for equal objects - the invariant, which we also can't enforce, user code might break this).

PS: I think that part of discussion rather belongs to the issue thread, which has some other arguments on why we want document this. See e.g. this.

dscorbett Oct 7, 2024

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Should the note warn against side effects in general (like I/O), not just mutation?

skirpichev Oct 7, 2024

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Should the note warn against side effects in general (like I/O), not just mutation?

Side effects, including in fact object mutation - are fine, unless they break idempotence.

But I think we don't loose something practically relevant if just forbid mutation of any objects in __bool__() (a shortened version of @ncoghlan suggestion):

Suggested change
Two successive calls to :meth:`!__bool__` on the same object must
return same value.
Calls to :meth:!__bool__` shouldn't mutate any objects.



.. _attribute-access:

Expand Down Expand Up @@ -2947,6 +2950,9 @@ through the object's keys; for sequences, it should iterate through the values.
:meth:`~object.__bool__` method and whose :meth:`!__len__` method returns zero is
considered to be false in a Boolean context.

Two successive calls to :meth:`!__len__` on the same object must
return same value.
Comment on lines +2953 to +2954

ncoghlan Oct 6, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality
Suggested change
Two successive calls to :meth:`!__len__` on the same object must
return same value.
Note: to help optimize logical expressions, implementations are permitted to assume
that calls to :meth:`!__len__` will not mutate that object, nor any other object.
While this expected invariant is not explicitly enforced, failing to abide
by it will result in implementation dependent runtime behaviour. This
implementation dependent behaviour may also be encountered when mutable
objects are shared across threads without appropriate synchronization.

skirpichev Oct 7, 2024

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Here is a typo (__len__ -> __bool__). But if we are going with this lengthly wording - I think it's better just point to the __bool__ docs.

.. impl-detail::

In CPython, the length is required to be at most :data:`sys.maxsize`.
Expand Down
Back | FazBrowse Home | New Git URL