[Docs] Clarifies ObjectIdentifier guarantees
#34349
Conversation
| /// `ObjectIdentifier` is only guaranteed to remain unique for the | ||
| /// lifetime of an object. When the instance gets deallocated, its object | ||
| /// identifier may be reused for a different object. (Internally, objects are | ||
| /// identified by their memory location.) |
There was a problem hiding this comment.
What does this parenthetical mean, and why is it important for users to know this?
There was a problem hiding this comment.
As I mentioned on the https://bugs.swift.org/browse/SR-13564, the result of this example is not as expected.
func makeIdentifier(input: Int) -> ObjectIdentifier {
ObjectIdentifier(NSNumber(value:input));
}
let a = makeIdentifier(input: Int.min);
let b = makeIdentifier(input: Int.max);
print(a == b) // print: true
There was a problem hiding this comment.
You're right, that result would be unexpected. However, I just ran your example, and on my machine it prints false, which is what I would expect.
Under what conditions are you getting true? I'm not sure I would understand how that comes about, and the parenthetical doesn't clarify for me.
There was a problem hiding this comment.
That example could give different results, depending on whether:
- tagged pointers are supported (Apple platforms only), or
- the allocator decides to reuse the same memory location.
(Although tagged pointers might not be used for Int.min and Int.max).
There was a problem hiding this comment.
You're right, that result would be unexpected. However, I just ran your example, and on my machine it prints
false, which is what I would expect.Under what conditions are you getting
true? I'm not sure I would understand how that comes about, and the parenthetical doesn't clarify for me.
Mac x86_64 & Xcode
| /// `ObjectIdentifier` is only guaranteed to remain unique for the | ||
| /// lifetime of an object. When the instance gets deallocated, its object | ||
| /// identifier may be reused for a different object. (Internally, objects are | ||
| /// identified by their memory location.) |
There was a problem hiding this comment.
Having wording for allocation/deallocation/memory locations here makes me a bit uncomfortable. That makes it seem like we are making some commitments about the memory allocation under the hood, but I'm not sure if we want to do that. How about something simpler:
The unique identity for a class instance is only valid for comparisons during
the lifetime of the instance. The unique identity for a metatype is always valid.
You could also omit the second sentence I suggested above if you think it's not helpful.
| /// `ObjectIdentifier` is only guaranteed to remain unique for the | ||
| /// lifetime of an object. |
There was a problem hiding this comment.
[Sorry if this seems like nit-picking but:] This doesn't sound quite right. ObjectIdentifier is a type. It is not the type which is unique, values of that type are unique at runtime. This is why I suggested:
The unique identity for a class instance is only valid for comparisons during
the lifetime of the instance.
The "unique identity" corresponds to values of type ObjectIdentifier, and IMO fits better stylistically with the rest of the doc comment, which also uses "unique identities" as shorthand for "values of type ObjectIdentifier".
Co-authored-by: Xiaodi Wu <[email protected]>
|
Thanks for identifying this issue and getting the docs updated, @sunbohong! |
|
@swift-ci Please smoke test |
SR-13564
rdar://69169351