575973 - GCWeakRef API should have refEqual(void *item) method

Status: RESOLVED WONTFIX

(Tamarin Graveyard :: Garbage Collection (mmGC), defect)

Description

[Felix S. Klock II [:pnkfelix, :fklock]]

When Steven did an audit of the player's uses of GCWeakRef::get(), he found a bunch of cases where it is only used to compare for equality with another pointer.

He correctly identified those cases as candidates for using peek() instead of get(); but, really the right thing is for GCWeakRef to offer an equality-predicate directly, rather than encouraging player code to invoke peek() directly.

Just one such predicate won't cover *all* of the cases that Steven found; in particular there's a place where the player was doing ((MethodClosure*)foo->peek())->atom() != closure->peek_savedThis().  But it would cover most of them, and seems like a sensible API addition.

Comment 1

[Lars T Hansen]

I gently vote for "equals" as the name of the predicate, it having a fairly obvious correspondence with the existing "isNull".  Perhaps we can have a poll.

Comment 2

[Felix S. Klock II [:pnkfelix, :fklock]]

(In reply to comment #1)
> I gently vote for "equals" as the name of the predicate, it having a fairly
> obvious correspondence with the existing "isNull".  Perhaps we can have a poll.

I thought I had a good argument against this, but I really don't.  (I had vague worries about people interpreting an equals() method to be a predicate on two GCWeakRef's; and worse yet, a void* argument type wouldn't stop them from writing such code.  But maybe this all goes back to "you can't idiot-proof against people who won't read documentation...")

Comment 3

[Sylvestre Ledru [:Sylvestre]]

Tamarin is a dead project now. Mass WONTFIX.

Comment 4

[Sylvestre Ledru [:Sylvestre]]

Tamarin isn't maintained anymore. WONTFIX remaining bugs.