| Summary: | DFG AbstractInterpreter: CheckArray filters array modes for DirectArguments only using NonArray | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Michael Saboff <msaboff> | ||||||
| Component: | JavaScriptCore | Assignee: | Michael Saboff <msaboff> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | ews-watchlist, keith_miller, mark.lam, saam, webkit-bug-importer | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
|
Description
Michael Saboff
2018-07-19 16:05:10 PDT
Actually, DirectArguments structure can have either a NonArray indexingType or a ArrayStorageShape. We need to allow for both possibilities. Created attachment 345466 [details]
Patch
Comment on attachment 345466 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=345466&action=review > Source/JavaScriptCore/ChangeLog:9 > + When filtering array modes for DirectArguments and ScopedArguments, we need to allow for the possibility that not ScopedArguments. > Source/JavaScriptCore/ChangeLog:10 > + they can either be NonArray or NonArrayWithArrayStorage (aka ArrayStorageShape). Might be worth a comment why this is. Also might worth noting why DirectArguments won't end up with Int32/Double/etc shapes. (I'd put that explanation here or in a comment in the code perhaps, or both) Actually, it might be worth checking out what ScopedArguments does for out of line arguments (the ones past the length of the arguments object). If like DirectArguments, it stores it in the Butterfly, you actually may need to handle ScopedArguments too (In reply to Saam Barati from comment #5) > Actually, it might be worth checking out what ScopedArguments does for out > of line arguments (the ones past the length of the arguments object). If > like DirectArguments, it stores it in the Butterfly, you actually may need > to handle ScopedArguments too As we discussed, I added the change for ScopedArguments and a test as well. Created attachment 345481 [details]
Updated patch that includes ScopedArguments change
Comment on attachment 345481 [details]
Updated patch that includes ScopedArguments change
r=me
Committed r234075: <https://trac.webkit.org/changeset/234075> |