[Qt] The QScriptValuePrivate::CSpecial is too generic.

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Jędrzej Nowacki]

The QScriptValuePrivate::CSpecial should be divided into a separate states; CNull and CUndefined.
It would simplify the QScriptValue's design and implementation which should use the m_state member for distinguish between states not m_value.

[Jędrzej Nowacki]

Created attachment 57663 [details]
Fix v1

No functional changes.

[Kenneth Rohde Christiansen]

Comment on attachment 57663 [details]
Fix v1

JavaScriptCore/qt/api/qscriptvalue_p.h:54
 +      CUndefined -> QSVP is undefined, but a JSC engine hasn't been associated yet.
So does this not change the behaviour of QtScript?

[Jędrzej Nowacki]

(In reply to comment #2)
> (From update of attachment 57663 [details])
> JavaScriptCore/qt/api/qscriptvalue_p.h:54
>  +      CUndefined -> QSVP is undefined, but a JSC engine hasn't been associated yet.
> So does this not change the behaviour of QtScript?

True, It is a small design fix :-) The CSpecial state was a bad idea. From the beginning CNull and CUndefined should be separated.

Patch removes conversions between double and enum.
> -    case CSpecial:
> -        return m_number == static_cast<int>(QScriptValue::NullValue);
> +    case CNull:
> +        return true;

Code is simpler, and there is no need to check other variable then m_state to find the real state of QScriptValuePrivate
> -    case CSpecial:
> -        return m_number == QScriptValue::NullValue ? QString::fromLatin1("null") : QString::fromLatin1("undefined");
> +    case CNull:
> +        return QString::fromLatin1("null");
> +    case CUndefined:
> +        return QString::fromLatin1("undefined");

I planed to change it in Feb. but the patch touches too many functions in the QSVP, so it was difficult to apply it without conflicts. This is the first time when a QSVP patches are not waiting in bugzilla's commit queue.

[WebKit Commit Bot]

Comment on attachment 57663 [details]
Fix v1

Clearing flags on attachment: 57663

Committed r60655: <http://trac.webkit.org/changeset/60655>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.