788199 - SpiderMonkey: Clarify and clean up getter/setterObject() vs. getter/setterValue() on shapes.

Status: RESOLVED DUPLICATE of bug 1700052

(Core :: JavaScript Engine, defect)

Description

[Kannan Vijayan [:djvj]]

No-one really seems to know how why this is supposed to work.  The following is describe for getters only but it also applies to setters.


The Issue:

BaseShape::hasGetterObject() [checks BaseShape::flags & HAS_GETTER_OBJECT]
Shape::hasGetterValue() [checks Shape::attrs & JSPROP_GETTER]

Shape::getterObject() - Asserts Shape::hasGetterValue(), returns base()->getterObj directly

Shape::getterValue() - Asserts Shape::hasGetterValue(), returns base->getterObj wrapping it in a Value, returning UndefinedValue if base()->getterObj is NULL.

BaseShape::getterObject() - Asserts BaseShape::hasGetterObject, returns getterObj.


The Questions:

What's the purpose of hasGetterObject?

Can shape->attr & JSPROP_GETTER ever be true at the same time shape->base()->flags & HAS_GETTER_OBJECT is false?

Why do the getterObject() methods on Shape vs. BaseShape check completely different conditions but return the same pointer?  Is one of these flags redundant?

Comment 1

[Kannan Vijayan [:djvj]]

A quick search on Aurora seems to indicate that the only uses of getterObject on BaseShape is from GC code, which does |hasGetterObject()| on baseShape, and then uses |getterObj| without any NULL checks.

Also, the only places baseShape->flags |= HAS_GETTER_OBJECT gets set is in jsscopeinlines.h, when the shape->attrs & JSPROP_GETTER is true, and the raw getter is not null.

The only user of hasGetterObject on baseShape is the GC.

All this leads to the conclusion that HAS_GETTER_OBJECT/hasGetterObject is effectively an elaborate null check.

Comment 2

[André Bargull [:anba]]

Fixed through bug 1700052.