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
bpo-35514: Enhanced the explanation on reference count details #11277
Conversation
Doc/c-api/intro.rst
Outdated
reference count of it, the function is said to *give* ownership of a new | ||
reference to its caller and the caller is said to *own* the reference. When a | ||
function returns an object without changing the reference count of it, the | ||
caller is said to *borrow* the reference. Nothing needs to be done for a | ||
borrowed reference. | ||
|
||
Conversely, when a calling function passes in a reference to an object, there | ||
are two possibilities: the function *steals* a reference to the object, or it |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you delete the double whitespace on .. *steals* a reference...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure I can
Asking @pablogsal for review as this seems closely related to his PyCon talk. |
Doc/c-api/intro.rst
Outdated
caller). Let's look at different types of reference ownership passings in more | ||
detail. | ||
|
||
When a function returns an object and effectively increases the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function does not increase the reference count when returning. Returning is just returning. The reference count is increased explicitly or implicitly on creation. Normally, a function returns something that someone else has created (and therefore increased the refcount by 1 - or alternatively is born with a refcount of 1). The function is transfering ownership of the existing refcount, not increasing it (as you mention somehow in the next sentences. I think this could be clearer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I knew that returning itself doesn't affect refcount and that is why I used the word "effectively".
I guess it can be clearer if I say
"When a function increases the reference count of an object and returns it, the function..."
But maybe it's not perfect yet.
I thought a function can increase the refcount of some object in a sense that the reference count can increase during that function calling. If I understood you correctly, I can't say a function increases or doesn't increase the refcount of some object because the function is only transferring the ownership of some existing object, right?
Do you think these are also wrong to say?
- When a function returns an object without changing the reference count of it, the caller is said to borrow the reference.
- (You comment below) The called function has two choices: Increase the refcount - take ownership / Don't increase the refcount
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hummm, I think there may be some confusion because I was not clear in my wording. My comment below applies only to the case when you pass something to a function and then the function can get ownership (increase the refcount) or stealing (don't increase the refcount).
This paragraph that we are commenting here applies to the case when the function returns. In that case, the function can tell you "here is my object, you can use it but is mine and can die unless you take ownership of it (increasing the refcount). But on the other side if you don't need it anymore you don't need to do something because is still mine" or conversely "here is a new object for you, you need to remember to decrease the reference when you no longer need it".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, now I see those two cases are different and let's only talk about this paragraph that we are commenting.
First, please be reminded of the purpose of this PR. If one reads this docs for the first time he has no idea what transferring of ownership actually means and he needs an explanation like this
Suppose a caller called a callee
- Owning a reference: the caller received an object from the callee which had effectively increased the reference count of the object it returned by 1 during function calling
- Borrowing a reference: the caller received an object from the callee which hadn't changed the reference count of the object it returned during function calling
Can you see why I keep talking about change of ownership in terms of increasing and decreasing of reference count now?
The only thing I want to make sure here is if it makes sense to say a function increases or doesn't increase the refcount of the object it returns. Can't I say PyLong_FromLong(1L)
increases the reference count of the object it returns, which is int(1)
? If not, how do you think I can explain it concretely? I can't simply say the caller receives an ownership of the object PyLong_FromLong
returns because it's only a circularly explanation and doesn't help readers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Owning a reference: the caller received an object from the callee which had effectively increased the reference count of the object it returned by 1 during function calling
But this is incomplete IMHO, you can receive some object via a borrow reference, increase it and now you own it. Ownership is linked to the duty of decreasing the reference when you are down, not on how did you obtain the object.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see what you mean. However, if I defined owning a a reference according to your suggestion, that concept would not be mutually exclusive with borrowing a reference. This may not be a good categorization and I expect it will only confuse readers.
I you don't like the word own, maybe we can use take instead because it has a more specific meaning in that it implies transferring of ownership. In addition, this doc also defines giving an ownership and it's more natural to talk about give and take than own and take.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Owning a reference is a contract (you are responsable to clean it) while borrowing a reference is a verb: is an action that ends with you not owning the reference. But then you can own it by increasing the reference count.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well I understand what you mean, but that was not the point of my argument. It seems this conversation is getting more confusing and I think I can just revert this part back to the old version.
Doc/c-api/intro.rst
Outdated
responsible for it any longer. | ||
Conversely, when a calling function passes in a reference to an object, there | ||
are two possibilities: the function *steals* a reference to the object, or it | ||
does not. If a called function decreases the reference count of an object, it |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not clear to me or is directly wrong. The called function has two choices:
- Increase the refcount - take ownership
- Don't increase the refcount - stealing ownership
In any case the function never decreases the reference count.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the correction
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pablogsal After studying some functions, I learned that functions that are considered to steal ownerships do decrease refcount. Take a look at this example PyTuple_SetItem
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After studying some functions, I learned that functions that are considered to steal ownerships do decrease refcount.
I have a reference, I passed to some function and it steals my reference by placing it into a tuple and not doing anything with my reference count. The function says that is "stealing" the reference: in this example, nobody is decrementing or incrementing anything. Now is somebody else's job to manage that object. In this example, this hypothetical function has stolen the reference from me without altering the refcount.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am really sorry for the hassle and I'm afraid I can't fully understand what you are saying here. However, I think I get this at least. "Stealing a reference does not guarantee instant decreasing in refcount". I will change the doc accordingly.
But please note that, for example, PyTuple_SetItem
does decrease the refcount of an passed-in object in some cases. I still think that stealing a reference can be understood as a contract thatPy_DECREF
would be called on the object of interest by the callee or anybody else who is not the caller in the future.
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase And if you don't make the requested changes, you will be put in the comfy chair! |
Thanks @csabella for the ping :) |
Since there was an additional commit after the last comments, I believe this PR is waiting for review. @pablogsal |
This has a merge conflict now. I'm not sure the change is necessary at all: the existing text seems clear to me, and we can't realistically address all possible user confusion. |
I agree. |
@birkenfeld
When I read the reference count details section of the docs first time, I found it hard to grasp what transferring of ownership means concretely, which is an important and repeating concept throughout the docs. Some parts were confusing. For example,
This part seems to explain what is to receive a new reference, in terms of ownership passing. It may have had to do the opposite, which is to explain what a transferring of ownership is in terms of reference count changes because transferring of ownership is a high level concept and changing a reference count is a concrete operation. I fixed the documentation based on this idea.
https://bugs.python.org/issue35514