[Webkit-unassigned] [Bug 98898] Partial support of 'typeof' node in DFG

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=98898





--- Comment #10 from Filip Pizlo <fpizlo at apple.com>  2012-10-26 01:53:32 PST ---
(From update of attachment 170602)
View in context: https://bugs.webkit.org/attachment.cgi?id=170602&action=review

I think you're almost there.  You just need a solution to *really weird* the object-is-undefined and object-is-function cases.

When you do so, be careful about the possibility that doing what jsTypeStringForValue() would cause side-effects.  In fact, I don't think you can do the whole getCallData() thing without a call frame (you don't really have a call frame in the JIT - I mean, I think we pass one in, but it's not really meant to be used because it can be in a pretty weird state since we're in the middle of OSR), and I'm pretty sure that it can have weird side effects.

> Source/JavaScriptCore/dfg/DFGAbstractState.cpp:699
> +    case Typeof: {
> +        JSValue child = forNode(node.child1()).value();
> +        if (child) {
> +            bool constantWasSet = false;
> +            SpeculatedType type = forNode(node.child1()).m_type;
> +            if (isObjectSpeculation(type))
> +                constantWasSet = trySetConstant(nodeIndex, jsString(&m_graph.m_globalData, String("object")));
> +            else if (isNumberSpeculation(type))
> +                constantWasSet = trySetConstant(nodeIndex, jsString(&m_graph.m_globalData, String("number")));
> +            else if (isStringSpeculation(type))
> +                constantWasSet = trySetConstant(nodeIndex, jsString(&m_graph.m_globalData, String("string")));
> +            else if (isFunctionSpeculation(type))
> +                constantWasSet = trySetConstant(nodeIndex, jsString(&m_graph.m_globalData, String("function")));
> +            else if (isBooleanSpeculation(type))
> +                constantWasSet = trySetConstant(nodeIndex, jsString(&m_graph.m_globalData, String("boolean")));
> +
> +            if (constantWasSet) {
> +                m_foundConstants = true;
> +                break;
> +            }
> +        }
> +        forNode(nodeIndex).set(SpecString);
> +        break;
> +    }

This is mostly right.  You've written it in the right form and style.  But, it has a bug: you're not matching the behavior of jsTypeStringForValue:

JSValue jsTypeStringForValue(CallFrame* callFrame, JSValue v)
{
    JSGlobalData& globalData = callFrame->globalData();
    if (v.isUndefined())
        return globalData.smallStrings.undefinedString(&globalData);
    if (v.isBoolean())
        return globalData.smallStrings.booleanString(&globalData);
    if (v.isNumber())
        return globalData.smallStrings.numberString(&globalData);
    if (v.isString())
        return globalData.smallStrings.stringString(&globalData);
    if (v.isObject()) {
        // Return "undefined" for objects that should be treated
        // as null when doing comparisons.
        if (asObject(v)->structure()->masqueradesAsUndefined(callFrame->lexicalGlobalObject()))
            return globalData.smallStrings.undefinedString(&globalData);
        CallData callData;
        JSObject* object = asObject(v);
        if (object->methodTable()->getCallData(object, callData) != CallTypeNone)
            return globalData.smallStrings.functionString(&globalData);
    }
    return globalData.smallStrings.objectString(&globalData);
}

In particular, if you have an object constant, you still need to check if it masquerades as undefined, or if it's callable.  Even things that aren't functions can pretend to be, by providing a call trap.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.