Summary: | test262: test262/test/built-ins/Object/prototype/toLocaleString/primitive_this_value.js | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Joseph Pecoraro <joepeck> | ||||||
Component: | JavaScriptCore | Assignee: | Joseph Pecoraro <joepeck> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | buildbot, commit-queue, joepeck, keith_miller, mark.lam, msaboff, saam | ||||||
Priority: | P2 | ||||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Joseph Pecoraro
2017-04-15 22:09:18 PDT
Created attachment 307217 [details]
[PATCH] Proposed Fix
I didn't add a new test for this because I considered it too obscure. test262 covers it, and I think that should be enough.
Comment on attachment 307217 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=307217&action=review > Source/JavaScriptCore/runtime/ObjectPrototype.cpp:294 > + JSValue toString = object->getV(exec, vm.propertyNames->toString, thisValue); We usually do this another way: We already have a way of distinguishing between |this| and receiver. The first argument to PropertySlot constructor is |this|. Then, you can call getPropertySlot on the receiver. That's what I'd do here instead of making this new function, which looks slightly wrong since it sets |this| only after doing the lookup. You can write code like this: PropertySlot slot(thisValue, ...Get); JSValue toString = object->getPropertySlot(exec, slot, toStringIdent) ? slot.getValue(exec, toStringIdent) : jsUndefined(); Comment on attachment 307217 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=307217&action=review >> Source/JavaScriptCore/runtime/ObjectPrototype.cpp:294 >> + JSValue toString = object->getV(exec, vm.propertyNames->toString, thisValue); > > We usually do this another way: > We already have a way of distinguishing between |this| and receiver. The first argument to PropertySlot constructor is |this|. Then, you can call getPropertySlot on the receiver. That's what I'd do here instead of making this new function, which looks slightly wrong since it sets |this| only after doing the lookup. You can write code like this: > > PropertySlot slot(thisValue, ...Get); > JSValue toString = object->getPropertySlot(exec, slot, toStringIdent) ? slot.getValue(exec, toStringIdent) : jsUndefined(); Much cleaner! Created attachment 307252 [details]
[PATCH] Proposed Fix
Comment on attachment 307252 [details] [PATCH] Proposed Fix Clearing flags on attachment: 307252 Committed r215405: <http://trac.webkit.org/changeset/215405> All reviewed patches have been landed. Closing bug. |