[webkit-reviews] review denied: [Bug 129563] DFG and FTL should specialize for and support CompareStrictEq over Misc (i.e. boolean, undefined, or null) : [Attachment 225596] the patch

[bugzilla-daemon at webkit.org]

Geoffrey Garen <ggaren at apple.com> has denied Filip Pizlo <fpizlo at apple.com>'s
request for review:
Bug 129563: DFG and FTL should specialize for and support CompareStrictEq over
Misc (i.e. boolean, undefined, or null)
https://bugs.webkit.org/show_bug.cgi?id=129563

Attachment 225596: the patch
https://bugs.webkit.org/attachment.cgi?id=225596&action=review

------- Additional Comments from Geoffrey Garen <ggaren at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=225596&action=review


> Source/JavaScriptCore/bytecode/SpeculatedType.cpp:403
> +SpeculatedType
leastUpperBoundOfStrictlyEquivalentSpeculations(SpeculatedType type)
> +{
> +    if (type & SpecInteger)
> +	   type |= SpecInteger;
> +    if (type & SpecString)
> +	   type |= SpecString;
> +    return type;
> +}

Maybe it's just Monday morning, but this function looks like a no-op. You test
the input type for SpecInteger, and then set SpecInteger if and only if it's
already set. Sam for SpecString. Can this be right?

> Source/JavaScriptCore/bytecode/SpeculatedType.h:79
> +static const SpeculatedType SpecMisc 	      = 0x30000000; // It's
definitely either a boolean, Null, or Undefined.

Looks like the convention here is to capitalize "Boolean".