| Summary: | REGRESSION(r271119) Check for nullness of preamble.caller | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Lauro Moura <lmoura> | ||||
| Component: | Tools / Tests | Assignee: | Lauro Moura <lmoura> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | alecflett, ashvayka, beidson, ews-watchlist, jsbell, webkit-bug-importer, ysuzuki | ||||
| Priority: | P2 | Keywords: | InRadar | ||||
| Version: | WebKit Nightly Build | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| See Also: | https://bugs.webkit.org/show_bug.cgi?id=220216 | ||||||
| Attachments: |
|
||||||
|
Description
Lauro Moura
2021-01-13 16:50:43 PST
Created attachment 417586 [details]
Patch
Comment on attachment 417586 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=417586&action=review Thank you for tracking this down! > LayoutTests/ChangeLog:8 > + r271119 changed some kinds of functions to return null when trying to What if partly revert r271119 by removing if (!function->jsExecutable()->hasCallerAndArgumentsProperties()) return JSValue::encode(jsNull()); from `caller` getter to completely align us with the other runtimes? (In reply to Alexey Shvayka from comment #2) > > What if partly revert r271119 by removing > > if (!function->jsExecutable()->hasCallerAndArgumentsProperties()) > return JSValue::encode(jsNull()); > > from `caller` getter to completely align us with the other runtimes? I tried it and most tests work, except function-hidden-as-caller.js, caller-property.js, and tail-call-recognize.js, as they would need to be updated to reflect the previous behavior. But wouldn't it go against the spec linked in the original revision[1]? Or, as there is still an open issue[2] about in TC39 github, should we try to mimic other runtimes while it's discussed there? (although no updates in the github issue since early last year) [1] https://tc39.es/ecma262/#sec-forbidden-extensions [2] https://github.com/tc39/ecma262/issues/562 Comment on attachment 417586 [details]
Patch
I think this is OK.
Comment on attachment 417586 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=417586&action=review >> LayoutTests/ChangeLog:8 >> + r271119 changed some kinds of functions to return null when trying to > > What if partly revert r271119 by removing > > if (!function->jsExecutable()->hasCallerAndArgumentsProperties()) > return JSValue::encode(jsNull()); > > from `caller` getter to completely align us with the other runtimes? Removing that part breaks the intent of https://trac.webkit.org/changeset/271119/webkit Comment on attachment 417586 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=417586&action=review >>> LayoutTests/ChangeLog:8 >>> + r271119 changed some kinds of functions to return null when trying to >> >> What if partly revert r271119 by removing >> >> if (!function->jsExecutable()->hasCallerAndArgumentsProperties()) >> return JSValue::encode(jsNull()); >> >> from `caller` getter to completely align us with the other runtimes? > > Removing that part breaks the intent of https://trac.webkit.org/changeset/271119/webkit Ah, no https://trac.webkit.org/changeset/230662/webkit Committed r272030: <https://trac.webkit.org/changeset/272030> All reviewed patches have been landed. Closing bug and clearing flags on attachment 417586 [details]. |