| Summary: | AccessCase::canReplace should allow a Getter to replace an IntrinsicGetter | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Tadeu Zagallo <tzagallo> | ||||||
| Component: | JavaScriptCore | Assignee: | Tadeu Zagallo <tzagallo> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | commit-queue, ews-watchlist, keith_miller, mark.lam, msaboff, saam, webkit-bug-importer | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
|
Description
Tadeu Zagallo
2020-03-16 16:36:00 PDT
Created attachment 393707 [details]
Patch
Comment on attachment 393707 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=393707&action=review > Source/JavaScriptCore/bytecode/AccessCase.cpp:681 > + if (other.type() != type() && other.type() != IntrinsicGetter) Isn't this just always false? I think you want an ||? Comment on attachment 393707 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=393707&action=review >> Source/JavaScriptCore/bytecode/AccessCase.cpp:681 >> + if (other.type() != type() && other.type() != IntrinsicGetter) > > Isn't this just always false? I think you want an ||? Nvm, I just misread this as ==. Comment on attachment 393707 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=393707&action=review r=me > Source/JavaScriptCore/ChangeLog:9 > + When we override an intrinsic getter with an user defined getter, we might end up with the "an user" => "a user" > Source/JavaScriptCore/bytecode/AccessCase.cpp:680 > + case Getter: You're missing the other direction here, where |this| is InrinsicGetter, and the other is Getter If you wanted to be fancy, you could have something like: bool isInterchangeable(AccessType a, AccessType b) { if (a == b) return true; if (a == Getter and b == IntrinsicGetter) return true; if (a == IntrinsicGettter and b == Getter) return true } and then just write this function in terms of that Comment on attachment 393707 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=393707&action=review >> Source/JavaScriptCore/ChangeLog:9 >> + When we override an intrinsic getter with an user defined getter, we might end up with the > > "an user" => "a user" I debated that, thanks. >> Source/JavaScriptCore/bytecode/AccessCase.cpp:680 >> + case Getter: > > You're missing the other direction here, where |this| is InrinsicGetter, and the other is Getter > > If you wanted to be fancy, you could have something like: > > bool isInterchangeable(AccessType a, AccessType b) > { > if (a == b) > return true; > if (a == Getter and b == IntrinsicGetter) > return true; > if (a == IntrinsicGettter and b == Getter) > return true > } > > and then just write this function in terms of that I was unsure whether it was possible to go in the other direction. Fixed it now. Created attachment 393773 [details]
Patch for landing
Comment on attachment 393773 [details] Patch for landing Clearing flags on attachment: 393773 Committed r258573: <https://trac.webkit.org/changeset/258573> All reviewed patches have been landed. Closing bug. |