[webkit-reviews] review denied: [Bug 115805] DFGArrayMode::fromObserved is too liberal when it sees different Array and NonArray shapes : [Attachment 201076] Patch

[bugzilla-daemon at webkit.org]

Filip Pizlo <fpizlo at apple.com> has denied Mark Hahnenberg
<mhahnenberg at apple.com>'s request for review:
Bug 115805: DFGArrayMode::fromObserved is too liberal when it sees different
Array and NonArray shapes
https://bugs.webkit.org/show_bug.cgi?id=115805

Attachment 201076: Patch
https://bugs.webkit.org/attachment.cgi?id=201076&action=review

------- Additional Comments from Filip Pizlo <fpizlo at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=201076&action=review


> Source/JavaScriptCore/dfg/DFGArrayMode.cpp:120
> -	   if (observed & (asArrayModes(ArrayWithUndecided) |
asArrayModes(ArrayWithInt32) | asArrayModes(ArrayWithDouble) |
asArrayModes(ArrayWithContiguous) | asArrayModes(ArrayWithArrayStorage) |
asArrayModes(ArrayWithSlowPutArrayStorage)))
> -	       arrayClass = Array::Array;
> -	   else if (observed & (asArrayModes(NonArray) |
asArrayModes(NonArrayWithInt32) | asArrayModes(NonArrayWithDouble) |
asArrayModes(NonArrayWithContiguous) | asArrayModes(NonArrayWithArrayStorage) |
asArrayModes(NonArrayWithSlowPutArrayStorage)))
> +	   if (observed & (asArrayModes(NonArray) |
asArrayModes(NonArrayWithInt32) | asArrayModes(NonArrayWithDouble) |
asArrayModes(NonArrayWithContiguous) | asArrayModes(NonArrayWithArrayStorage) |
asArrayModes(NonArrayWithSlowPutArrayStorage)))
>	       arrayClass = Array::NonArray;
> +	   else if (observed & (asArrayModes(ArrayWithUndecided) |
asArrayModes(ArrayWithInt32) | asArrayModes(ArrayWithDouble) |
asArrayModes(ArrayWithContiguous) | asArrayModes(ArrayWithArrayStorage) |
asArrayModes(ArrayWithSlowPutArrayStorage)))
> +	       arrayClass = Array::Array;
>	   else
>	       arrayClass = Array::PossiblyArray;

Actually, I think the bug here is that PossiblyArray wasn't working right. 
PossiblyArray is supposed to represent "this could be an Array or a NonArray". 
So it appears that the problem here is that PossiblyArray isn't being compiled
to do the right thing (i.e. accept either Array or NonArray).